[PATCH] drm/nouveau/kms: Implement KDB debug hooks for nouveau KMS.

[PATCH] drm/nouveau/kms: Implement KDB debug hooks for nouveau KMS.

[Chris Ball]

Hi,

Here's a patch to add support for KMS debugging to Nouveau, along the
style of the previous patches for Intel¹ and Radeon².  I'm only able
to test on nv50 here, so a test on nv04 would be much appreciated,
and I've published instructions on how to test here³.  Thanks!

- Chris.

¹:  http://lkml.org/lkml/2010/8/5/280
²:  http://dev.laptop.org/~cjb/kdb-radeon.patch
³:  http://blog.printf.net/articles/2010/08/19/kdb-kms-for-nouveau-radeon


From: Chris Ball <cjb@laptop.org>
Date: Thu, 19 Aug 2010 12:55:34 -0400
Subject: [PATCH] drm/nouveau/kms: Implement KDB debug hooks for nouveau KMS.

Signed-off-by: Chris Ball <cjb@laptop.org>
---
 drivers/gpu/drm/nouveau/nouveau_fbcon.c |    6 ++++
 drivers/gpu/drm/nouveau/nv04_crtc.c     |   46 ++++++++++++++++++++++++-----
 drivers/gpu/drm/nouveau/nv50_crtc.c     |   49 ++++++++++++++++++++++--------
 3 files changed, 80 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index 2fb2444..aac1460 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -104,6 +104,8 @@ static struct fb_ops nouveau_fbcon_ops = {
 	.fb_pan_display = drm_fb_helper_pan_display,
 	.fb_blank = drm_fb_helper_blank,
 	.fb_setcmap = drm_fb_helper_setcmap,
+	.fb_debug_enter = drm_fb_helper_debug_enter,
+	.fb_debug_leave = drm_fb_helper_debug_leave,
 };
 
 static struct fb_ops nv04_fbcon_ops = {
@@ -117,6 +119,8 @@ static struct fb_ops nv04_fbcon_ops = {
 	.fb_pan_display = drm_fb_helper_pan_display,
 	.fb_blank = drm_fb_helper_blank,
 	.fb_setcmap = drm_fb_helper_setcmap,
+	.fb_debug_enter = drm_fb_helper_debug_enter,
+	.fb_debug_leave = drm_fb_helper_debug_leave,
 };
 
 static struct fb_ops nv50_fbcon_ops = {
@@ -130,6 +134,8 @@ static struct fb_ops nv50_fbcon_ops = {
 	.fb_pan_display = drm_fb_helper_pan_display,
 	.fb_blank = drm_fb_helper_blank,
 	.fb_setcmap = drm_fb_helper_setcmap,
+	.fb_debug_enter = drm_fb_helper_debug_enter,
+	.fb_debug_leave = drm_fb_helper_debug_leave,
 };
 
 static void nouveau_fbcon_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
diff --git a/drivers/gpu/drm/nouveau/nv04_crtc.c b/drivers/gpu/drm/nouveau/nv04_crtc.c
index 1c20c08..4205daf 100644
--- a/drivers/gpu/drm/nouveau/nv04_crtc.c
+++ b/drivers/gpu/drm/nouveau/nv04_crtc.c
@@ -767,8 +767,9 @@ nv_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b, uint32_t size)
 }
 
 static int
-nv04_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
-			struct drm_framebuffer *old_fb)
+nv04_crtc_do_mode_set_base(struct drm_crtc *crtc,
+			   struct drm_framebuffer *passed_fb,
+			   int x, int y, bool atomic)
 {
 	struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
@@ -779,13 +780,26 @@ nv04_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 	int arb_burst, arb_lwm;
 	int ret;
 
-	ret = nouveau_bo_pin(fb->nvbo, TTM_PL_FLAG_VRAM);
-	if (ret)
-		return ret;
+	/* If atomic, we want to switch to the fb we were passed, so
+	 * now we update pointers to do that.  (We don't pin; just
+	 * assume we're already pinned and update the base address.)
+	 */
+	if (atomic) {
+		drm_fb = passed_fb;
+		fb = nouveau_framebuffer(passed_fb);
+	}
+	else {
+		/* If not atomic, we can go ahead and pin, and unpin the
+		 * old fb we were passed.
+		 */
+		ret = nouveau_bo_pin(fb->nvbo, TTM_PL_FLAG_VRAM);
+		if (ret)
+			return ret;
 
-	if (old_fb) {
-		struct nouveau_framebuffer *ofb = nouveau_framebuffer(old_fb);
-		nouveau_bo_unpin(ofb->nvbo);
+		if (passed_fb) {
+			struct nouveau_framebuffer *ofb = nouveau_framebuffer(passed_fb);
+			nouveau_bo_unpin(ofb->nvbo);
+		}
 	}
 
 	nv_crtc->fb.offset = fb->nvbo->bo.offset;
@@ -833,6 +847,21 @@ nv04_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 	return 0;
 }
 
+static int
+nv04_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
+			struct drm_framebuffer *old_fb)
+{
+	return nv04_crtc_do_mode_set_base(crtc, old_fb, x, y, false);
+}
+
+static int
+nv04_crtc_mode_set_base_atomic(struct drm_crtc *crtc,
+			       struct drm_framebuffer *fb,
+			       int x, int y)
+{
+	return nv04_crtc_do_mode_set_base(crtc, fb, x, y, true);
+}
+
 static void nv04_cursor_upload(struct drm_device *dev, struct nouveau_bo *src,
 			       struct nouveau_bo *dst)
 {
@@ -961,6 +990,7 @@ static const struct drm_crtc_helper_funcs nv04_crtc_helper_funcs = {
 	.mode_fixup = nv_crtc_mode_fixup,
 	.mode_set = nv_crtc_mode_set,
 	.mode_set_base = nv04_crtc_mode_set_base,
+	.mode_set_base_atomic = nv04_crtc_mode_set_base_atomic,
 	.load_lut = nv_crtc_gamma_load,
 };
 
diff --git a/drivers/gpu/drm/nouveau/nv50_crtc.c b/drivers/gpu/drm/nouveau/nv50_crtc.c
index 5d11ea1..9ec7952 100644
--- a/drivers/gpu/drm/nouveau/nv50_crtc.c
+++ b/drivers/gpu/drm/nouveau/nv50_crtc.c
@@ -477,8 +477,9 @@ nv50_crtc_mode_fixup(struct drm_crtc *crtc, struct drm_display_mode *mode,
 }
 
 static int
-nv50_crtc_do_mode_set_base(struct drm_crtc *crtc, int x, int y,
-			   struct drm_framebuffer *old_fb, bool update)
+nv50_crtc_do_mode_set_base(struct drm_crtc *crtc,
+			   struct drm_framebuffer *passed_fb,
+			   int x, int y, bool update, bool atomic)
 {
 	struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
 	struct drm_device *dev = nv_crtc->base.dev;
@@ -490,6 +491,28 @@ nv50_crtc_do_mode_set_base(struct drm_crtc *crtc, int x, int y,
 
 	NV_DEBUG_KMS(dev, "index %d\n", nv_crtc->index);
 
+	/* If atomic, we want to switch to the fb we were passed, so
+	 * now we update pointers to do that.  (We don't pin; just
+	 * assume we're already pinned and update the base address.)
+	 */
+	if (atomic) {
+		drm_fb = passed_fb;
+		fb = nouveau_framebuffer(passed_fb);
+	}
+	else {
+		/* If not atomic, we can go ahead and pin, and unpin the
+		 * old fb we were passed.
+		 */
+		ret = nouveau_bo_pin(fb->nvbo, TTM_PL_FLAG_VRAM);
+		if (ret)
+			return ret;
+
+		if (passed_fb) {
+			struct nouveau_framebuffer *ofb = nouveau_framebuffer(passed_fb);
+			nouveau_bo_unpin(ofb->nvbo);
+		}
+	}
+
 	switch (drm_fb->depth) {
 	case  8:
 		format = NV50_EVO_CRTC_FB_DEPTH_8;
@@ -512,15 +535,6 @@ nv50_crtc_do_mode_set_base(struct drm_crtc *crtc, int x, int y,
 		 return -EINVAL;
 	}
 
-	ret = nouveau_bo_pin(fb->nvbo, TTM_PL_FLAG_VRAM);
-	if (ret)
-		return ret;
-
-	if (old_fb) {
-		struct nouveau_framebuffer *ofb = nouveau_framebuffer(old_fb);
-		nouveau_bo_unpin(ofb->nvbo);
-	}
-
 	nv_crtc->fb.offset = fb->nvbo->bo.offset - dev_priv->vm_vram_base;
 	nv_crtc->fb.tile_flags = fb->nvbo->tile_flags;
 	nv_crtc->fb.cpp = drm_fb->bits_per_pixel / 8;
@@ -671,14 +685,22 @@ nv50_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode,
 	nv_crtc->set_dither(nv_crtc, nv_connector->use_dithering, false);
 	nv_crtc->set_scale(nv_crtc, nv_connector->scaling_mode, false);
 
-	return nv50_crtc_do_mode_set_base(crtc, x, y, old_fb, false);
+	return nv50_crtc_do_mode_set_base(crtc, old_fb, x, y, false, false);
 }
 
 static int
 nv50_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 			struct drm_framebuffer *old_fb)
 {
-	return nv50_crtc_do_mode_set_base(crtc, x, y, old_fb, true);
+	return nv50_crtc_do_mode_set_base(crtc, old_fb, x, y, true, false);
+}
+
+static int
+nv50_crtc_mode_set_base_atomic(struct drm_crtc *crtc,
+			       struct drm_framebuffer *fb,
+			       int x, int y)
+{
+	return nv50_crtc_do_mode_set_base(crtc, fb, x, y, true, true);
 }
 
 static const struct drm_crtc_helper_funcs nv50_crtc_helper_funcs = {
@@ -688,6 +710,7 @@ static const struct drm_crtc_helper_funcs nv50_crtc_helper_funcs = {
 	.mode_fixup = nv50_crtc_mode_fixup,
 	.mode_set = nv50_crtc_mode_set,
 	.mode_set_base = nv50_crtc_mode_set_base,
+	.mode_set_base_atomic = nv50_crtc_mode_set_base_atomic,
 	.load_lut = nv50_crtc_lut_load,
 };
 
-- 
1.6.2.5

------------------------------------------------------------------------------
This SF.net email is sponsored by 

Make an app they can't live without
Enter the BlackBerry Developer Challenge
http://p.sf.net/sfu/RIM-dev2dev 
_______________________________________________
Kgdb-bugreport mailing list
Kgdb-bugreport@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport

Re: [PATCH] drm/nouveau/kms: Implement KDB debug hooks for nouveau KMS.

[Francisco Jerez]

[-- Attachment #1.1.1: Type: text/plain, Size: 967 bytes --]

Chris Ball <cjb-2X9k7bc8m7Mdnm+yROfE0A@public.gmane.org> writes:

> Hi,
>
> Here's a patch to add support for KMS debugging to Nouveau, along the
> style of the previous patches for Intel¹ and Radeon².  I'm only able
> to test on nv50 here, so a test on nv04 would be much appreciated,
> and I've published instructions on how to test here³.  Thanks!
>
There is a problem with this on pre-nv20 cards. Fbcon acceleration won't
work properly with IRQs disabled because you miss the context switching
interrupts: You'll get a locked up fbcon if you hit sysrq-g when there's
some process using the GPU.

I'd suggest disabling acceleration while in debug mode (e.g. using
nouveau_fbcon_save_disable_accel()). That aside the patch looks good to
me.

> - Chris.
>
> ¹:  http://lkml.org/lkml/2010/8/5/280
> ²:  http://dev.laptop.org/~cjb/kdb-radeon.patch
> ³:  http://blog.printf.net/articles/2010/08/19/kdb-kms-for-nouveau-radeon
>
>
> [...]

[-- Attachment #1.2: Type: application/pgp-signature, Size: 229 bytes --]

[-- Attachment #2: Type: text/plain, Size: 181 bytes --]

_______________________________________________
Nouveau mailing list
Nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH] drm/nouveau/kms: Implement KDB debug hooks for nouveau KMS.

[Chris Ball]

Hi Francisco,

   > There is a problem with this on pre-nv20 cards. Fbcon
   > acceleration won't work properly with IRQs disabled because you
   > miss the context switching interrupts: You'll get a locked up
   > fbcon if you hit sysrq-g when there's some process using the GPU.
   > 
   > I'd suggest disabling acceleration while in debug mode
   > (e.g. using nouveau_fbcon_save_disable_accel()). That aside the
   > patch looks good to me.

Thanks very much for this.  Here's a (only compile-tested) patch for
this, on top of jwessel's current kgdb-next branch.  Jason, would you
mind testing on pre-nv20 and applying?

Thanks,

- Chris.


From: Chris Ball <cjb@laptop.org>
Subject: [PATCH] drm/nouveau/kms: Avoid a hang entering KDB with VT accel on.

Francisco Jerez advises that pre-nv20 cards would hang if we entered
kdb with accel on and IRQs disabled, so we now disable accel before
entering kdb and re-enable it on the way back out.

Signed-off-by: Chris Ball <cjb@laptop.org>
Cc: Francisco Jerez <currojerez@riseup.net>
Cc: Jason Wessel <jason.wessel@windriver.com>
---
 drivers/gpu/drm/nouveau/nv04_crtc.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nv04_crtc.c b/drivers/gpu/drm/nouveau/nv04_crtc.c
index fb669dd..427f90e 100644
--- a/drivers/gpu/drm/nouveau/nv04_crtc.c
+++ b/drivers/gpu/drm/nouveau/nv04_crtc.c
@@ -31,10 +31,11 @@
 #include "nouveau_connector.h"
 #include "nouveau_crtc.h"
 #include "nouveau_fb.h"
 #include "nouveau_hw.h"
 #include "nvreg.h"
+#include "nouveau_fbcon.h"
 
 static int
 nv04_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 			struct drm_framebuffer *old_fb);
 
@@ -858,10 +859,18 @@ nv04_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 static int
 nv04_crtc_mode_set_base_atomic(struct drm_crtc *crtc,
 			       struct drm_framebuffer *fb,
 			       int x, int y, int enter)
 {
+	struct drm_nouveau_private *dev_priv = crtc->dev->dev_private;
+	struct drm_device *dev = dev_priv->dev;
+
+	if (enter)
+		nouveau_fbcon_save_disable_accel(dev);
+	else
+		nouveau_fbcon_restore_accel(dev);
+
 	return nv04_crtc_do_mode_set_base(crtc, fb, x, y, true);
 }
 
 static void nv04_cursor_upload(struct drm_device *dev, struct nouveau_bo *src,
 			       struct nouveau_bo *dst)
-- 
1.6.2.5

------------------------------------------------------------------------------
Sell apps to millions through the Intel(R) Atom(Tm) Developer Program
Be part of this innovative community and reach millions of netbook users 
worldwide. Take advantage of special opportunities to increase revenue and 
speed time-to-market. Join now, and jumpstart your future.
http://p.sf.net/sfu/intel-atom-d2d

Re: [PATCH] drm/nouveau/kms: Implement KDB debug hooks for nouveau KMS.

[Francisco Jerez]

[-- Attachment #1.1.1: Type: text/plain, Size: 2819 bytes --]

Chris Ball <cjb-2X9k7bc8m7Mdnm+yROfE0A@public.gmane.org> writes:

> Hi Francisco,
>
>    > There is a problem with this on pre-nv20 cards. Fbcon
>    > acceleration won't work properly with IRQs disabled because you
>    > miss the context switching interrupts: You'll get a locked up
>    > fbcon if you hit sysrq-g when there's some process using the GPU.
>    > 
>    > I'd suggest disabling acceleration while in debug mode
>    > (e.g. using nouveau_fbcon_save_disable_accel()). That aside the
>    > patch looks good to me.
>
> Thanks very much for this.  Here's a (only compile-tested) patch for
> this, on top of jwessel's current kgdb-next branch.  Jason, would you
> mind testing on pre-nv20 and applying?
>
Thanks, it looks OK to me, and I just tested it successfully on
nv11. You can have my "Acked-by" on both nouveau patches if you need it.

> Thanks,
>
> - Chris.
>
>
> From: Chris Ball <cjb-2X9k7bc8m7Mdnm+yROfE0A@public.gmane.org>
> Subject: [PATCH] drm/nouveau/kms: Avoid a hang entering KDB with VT accel on.
>
> Francisco Jerez advises that pre-nv20 cards would hang if we entered
> kdb with accel on and IRQs disabled, so we now disable accel before
> entering kdb and re-enable it on the way back out.
>
> Signed-off-by: Chris Ball <cjb-2X9k7bc8m7Mdnm+yROfE0A@public.gmane.org>
> Cc: Francisco Jerez <currojerez-sGOZH3hwPm2sTnJN9+BGXg@public.gmane.org>
> Cc: Jason Wessel <jason.wessel-CWA4WttNNZF54TAoqtyWWQ@public.gmane.org>
> ---
>  drivers/gpu/drm/nouveau/nv04_crtc.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nv04_crtc.c b/drivers/gpu/drm/nouveau/nv04_crtc.c
> index fb669dd..427f90e 100644
> --- a/drivers/gpu/drm/nouveau/nv04_crtc.c
> +++ b/drivers/gpu/drm/nouveau/nv04_crtc.c
> @@ -31,10 +31,11 @@
>  #include "nouveau_connector.h"
>  #include "nouveau_crtc.h"
>  #include "nouveau_fb.h"
>  #include "nouveau_hw.h"
>  #include "nvreg.h"
> +#include "nouveau_fbcon.h"
>  
>  static int
>  nv04_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
>  			struct drm_framebuffer *old_fb);
>  
> @@ -858,10 +859,18 @@ nv04_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
>  static int
>  nv04_crtc_mode_set_base_atomic(struct drm_crtc *crtc,
>  			       struct drm_framebuffer *fb,
>  			       int x, int y, int enter)
>  {
> +	struct drm_nouveau_private *dev_priv = crtc->dev->dev_private;
> +	struct drm_device *dev = dev_priv->dev;
> +
> +	if (enter)
> +		nouveau_fbcon_save_disable_accel(dev);
> +	else
> +		nouveau_fbcon_restore_accel(dev);
> +
>  	return nv04_crtc_do_mode_set_base(crtc, fb, x, y, true);
>  }
>  
>  static void nv04_cursor_upload(struct drm_device *dev, struct nouveau_bo *src,
>  			       struct nouveau_bo *dst)

[-- Attachment #1.2: Type: application/pgp-signature, Size: 229 bytes --]

[-- Attachment #2: Type: text/plain, Size: 181 bytes --]

_______________________________________________
Nouveau mailing list
Nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH] drm/nouveau/kms: Implement KDB debug hooks for nouveau KMS.

[Jason Wessel]

On 08/23/2010 03:50 PM, Chris Ball wrote:
> Hi Francisco,
>
>    > There is a problem with this on pre-nv20 cards. Fbcon
>    > acceleration won't work properly with IRQs disabled because you
>    > miss the context switching interrupts: You'll get a locked up
>    > fbcon if you hit sysrq-g when there's some process using the GPU.
>    > 
>    > I'd suggest disabling acceleration while in debug mode
>    > (e.g. using nouveau_fbcon_save_disable_accel()). That aside the
>    > patch looks good to me.
>
> Thanks very much for this.  Here's a (only compile-tested) patch for
> this, on top of jwessel's current kgdb-next branch.  Jason, would you
> mind testing on pre-nv20 and applying?
>
>   
Thanks Chris,

This is applied to the kgdb tree, and I'll send the whole atomic
modesetting series over to dri-devel as soon as it passes the regression
tests as that should be the place it is merged from unless we receive
sign-off from Jesse Barnes and Dave Airlie in which case I'll put it in
a future pull request.

It was pretty cool to see the kdb/kms feature finally working on the
laptop I use the most.

Thanks,
Jason.

------------------------------------------------------------------------------
Start uncovering the many advantages of virtual appliances
and start using them to simplify application deployment and
accelerate your shift to cloud computing.
http://p.sf.net/sfu/novell-sfdev2dev

Re: [PATCH] drm/nouveau/kms: Implement KDB debug hooks for nouveau KMS.

[Maxim Levitsky]

On Thu, 2010-08-19 at 13:55 -0400, Chris Ball wrote: 
> Hi,
> 
> Here's a patch to add support for KMS debugging to Nouveau, along the
> style of the previous patches for Intel¹ and Radeon².  I'm only able
> to test on nv50 here, so a test on nv04 would be much appreciated,
> and I've published instructions on how to test here³.  Thanks!
> 
> - Chris.

Hi,

I just tried that patch, but unfortunately nether with nor without it
kdb seems not to work.
It could be id10t error from my side, but I did test the kdb in the past
with few KMS patches, and it seemed to work.

Now I can't even get its prompt on the console.

This is what I do:

echo kbd | sudo tee /sys/module/kgdboc/parameters/kgdboc
(also tried booting with kgdboc=kbd)

Both seems to start kdb, because

maxim@maxim-laptop:~$ cat /sys/module/kgdboc/parameters/kgdboc 
kbd

maxim@maxim-laptop:~$ dmesg | grep kgdb
[    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-2.6.36-rc3+ root=UUID=52341b68-74f3-4c96-aaf8-7586a06c4b4e ro splash video=nouveau nmi_watchdog=lapic kgdboc=kbd
[    0.000000] Kernel command line: BOOT_IMAGE=/boot/vmlinuz-2.6.36-rc3+ root=UUID=52341b68-74f3-4c96-aaf8-7586a06c4b4e ro splash video=nouveau nmi_watchdog=lapic kgdboc=kbd
[    2.671914] kgdb: Registered I/O driver kgdboc.


Now when I first switch to console and then press Alt+SysRQ+G, I just get SYSRQ: DEBUG, and system hangs.
It could be printk level, but that should be ok, no?

maxim@maxim-laptop:~$ cat /proc/sys/kernel/printk
7       4       1       7


Also if I remember correct inputting 'g' then ENTER should resume the system, but it doesn't.

So I think kdb doesn't work at all here

Best regards,
        Maxim Levitsky



_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [Nouveau] [PATCH] drm/nouveau/kms: Implement KDB debug hooks for nouveau KMS.

[Jason Wessel]

On 09/01/2010 04:56 AM, Maxim Levitsky wrote:
> On Thu, 2010-08-19 at 13:55 -0400, Chris Ball wrote: 
>   
>> Hi,
>>
>> Here's a patch to add support for KMS debugging to Nouveau, along the
>> style of the previous patches for Intel¹ and Radeon².  I'm only able
>> to test on nv50 here, so a test on nv04 would be much appreciated,
>> and I've published instructions on how to test here³.  Thanks!
>>
>> - Chris.
>>     
>
> Hi,
>
> I just tried that patch, but unfortunately nether with nor without it
> kdb seems not to work.
> It could be id10t error from my side, but I did test the kdb in the past
> with few KMS patches, and it seemed to work.
>
> Now I can't even get its prompt on the console.
>
> This is what I do:
>
> echo kbd | sudo tee /sys/module/kgdboc/parameters/kgdboc
> (also tried booting with kgdboc=kbd)
>   

Try changing it to kgdboc=kms,kbd  or the "echo kms,kbd"

When you use only the kbd, the kms feature is not activated.

Jason.



------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd
_______________________________________________
Kgdb-bugreport mailing list
Kgdb-bugreport@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport

Re: [Nouveau] [PATCH] drm/nouveau/kms: Implement KDB debug hooks for nouveau KMS.

[Maxim Levitsky]

On Wed, 2010-09-01 at 06:35 -0500, Jason Wessel wrote: 
> On 09/01/2010 04:56 AM, Maxim Levitsky wrote:
> > On Thu, 2010-08-19 at 13:55 -0400, Chris Ball wrote: 
> >   
> >> Hi,
> >>
> >> Here's a patch to add support for KMS debugging to Nouveau, along the
> >> style of the previous patches for Intel¹ and Radeon².  I'm only able
> >> to test on nv50 here, so a test on nv04 would be much appreciated,
> >> and I've published instructions on how to test here³.  Thanks!
> >>
> >> - Chris.
> >>     
> >
> > Hi,
> >
> > I just tried that patch, but unfortunately nether with nor without it
> > kdb seems not to work.
> > It could be id10t error from my side, but I did test the kdb in the past
> > with few KMS patches, and it seemed to work.
> >
> > Now I can't even get its prompt on the console.
> >
> > This is what I do:
> >
> > echo kbd | sudo tee /sys/module/kgdboc/parameters/kgdboc
> > (also tried booting with kgdboc=kbd)
> >   
> 
> Try changing it to kgdboc=kms,kbd  or the "echo kms,kbd"
> 
> When you use only the kbd, the kms feature is not activated.
This doesn't help.

I am afraid that this bug isn't related to kms, but rather is generic.

Best regards,
Maxim Levitsky



------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:

Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd
_______________________________________________
Kgdb-bugreport mailing list
Kgdb-bugreport@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport

Re: [Nouveau] [PATCH] drm/nouveau/kms: Implement KDB debug hooks for nouveau KMS.

[Maxim Levitsky]

On Thu, 2010-09-02 at 13:46 +0300, Maxim Levitsky wrote: 
> On Wed, 2010-09-01 at 06:35 -0500, Jason Wessel wrote: 
> > On 09/01/2010 04:56 AM, Maxim Levitsky wrote:
> > > On Thu, 2010-08-19 at 13:55 -0400, Chris Ball wrote: 
> > >   
> > >> Hi,
> > >>
> > >> Here's a patch to add support for KMS debugging to Nouveau, along the
> > >> style of the previous patches for Intel¹ and Radeon².  I'm only able
> > >> to test on nv50 here, so a test on nv04 would be much appreciated,
> > >> and I've published instructions on how to test here³.  Thanks!
> > >>
> > >> - Chris.
> > >>     
> > >
> > > Hi,
> > >
> > > I just tried that patch, but unfortunately nether with nor without it
> > > kdb seems not to work.
> > > It could be id10t error from my side, but I did test the kdb in the past
> > > with few KMS patches, and it seemed to work.
> > >
> > > Now I can't even get its prompt on the console.
> > >
> > > This is what I do:
> > >
> > > echo kbd | sudo tee /sys/module/kgdboc/parameters/kgdboc
> > > (also tried booting with kgdboc=kbd)
> > >   
> > 
> > Try changing it to kgdboc=kms,kbd  or the "echo kms,kbd"
> > 
> > When you use only the kbd, the kms feature is not activated.
> This doesn't help.
> 
> I am afraid that this bug isn't related to kms, but rather is generic.

I turns out that it was the NMI watchdog that I had enabled.
Without it kdb works very well, including the kms support.

It would be better if you were to detect kms instead of adding an
explicit param to kgdboc cmd line.

Also found out that after a debug session with Alt+SysRQ+g and X
running, these keys aren't released. I had to press on all of them to
make them release.
It makes sense as kgdboc in that case reads directly from keyboard port.

Best regards,
Maxim Levitsky




------------------------------------------------------------------------------
Start uncovering the many advantages of virtual appliances
and start using them to simplify application deployment and
accelerate your shift to cloud computing.
http://p.sf.net/sfu/novell-sfdev2dev
_______________________________________________
Kgdb-bugreport mailing list
Kgdb-bugreport@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport

Re: [Nouveau] [PATCH] drm/nouveau/kms: Implement KDB debug hooks for nouveau KMS.

[Maxim Levitsky]

On Wed, 2010-09-22 at 03:42 +0200, Maxim Levitsky wrote: 
> On Thu, 2010-09-02 at 13:46 +0300, Maxim Levitsky wrote: 
> > On Wed, 2010-09-01 at 06:35 -0500, Jason Wessel wrote: 
> > > On 09/01/2010 04:56 AM, Maxim Levitsky wrote:
> > > > On Thu, 2010-08-19 at 13:55 -0400, Chris Ball wrote: 
> > > >   
> > > >> Hi,
> > > >>
> > > >> Here's a patch to add support for KMS debugging to Nouveau, along the
> > > >> style of the previous patches for Intel¹ and Radeon².  I'm only able
> > > >> to test on nv50 here, so a test on nv04 would be much appreciated,
> > > >> and I've published instructions on how to test here³.  Thanks!
> > > >>
> > > >> - Chris.
> > > >>     
> > > >
> > > > Hi,
> > > >
> > > > I just tried that patch, but unfortunately nether with nor without it
> > > > kdb seems not to work.
> > > > It could be id10t error from my side, but I did test the kdb in the past
> > > > with few KMS patches, and it seemed to work.
> > > >
> > > > Now I can't even get its prompt on the console.
> > > >
> > > > This is what I do:
> > > >
> > > > echo kbd | sudo tee /sys/module/kgdboc/parameters/kgdboc
> > > > (also tried booting with kgdboc=kbd)
> > > >   
> > > 
> > > Try changing it to kgdboc=kms,kbd  or the "echo kms,kbd"
> > > 
> > > When you use only the kbd, the kms feature is not activated.
> > This doesn't help.
> > 
> > I am afraid that this bug isn't related to kms, but rather is generic.
> 
> I turns out that it was the NMI watchdog that I had enabled.
> Without it kdb works very well, including the kms support.
Please disregard this. kdb works with nmi watchdog now as well.
Probably something was fixed, maybe unrelated to it.

> 
> It would be better if you were to detect kms instead of adding an
> explicit param to kgdboc cmd line.
> 
> Also found out that after a debug session with Alt+SysRQ+g and X
> running, these keys aren't released. I had to press on all of them to
> make them release.
> It makes sense as kgdboc in that case reads directly from keyboard port.
And I see that kgdb actually has a code that works that around.
I suspect that what happens is that keys are released before X continues
running, and therefore it doesn't pick these events up.


Best regards,
	Maxim Levitsky




------------------------------------------------------------------------------
Start uncovering the many advantages of virtual appliances
and start using them to simplify application deployment and
accelerate your shift to cloud computing.
http://p.sf.net/sfu/novell-sfdev2dev
_______________________________________________
Kgdb-bugreport mailing list
Kgdb-bugreport@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport

Re: [Nouveau] [PATCH] drm/nouveau/kms: Implement KDB debug hooks for nouveau KMS.

[Maxim Levitsky]

On Wed, 2010-09-22 at 16:03 +0200, Maxim Levitsky wrote: 
> On Wed, 2010-09-22 at 03:42 +0200, Maxim Levitsky wrote: 
> > On Thu, 2010-09-02 at 13:46 +0300, Maxim Levitsky wrote: 
> > > On Wed, 2010-09-01 at 06:35 -0500, Jason Wessel wrote: 
> > > > On 09/01/2010 04:56 AM, Maxim Levitsky wrote:
> > > > > On Thu, 2010-08-19 at 13:55 -0400, Chris Ball wrote: 
> > > > >   
> > > > >> Hi,
> > > > >>
> > > > >> Here's a patch to add support for KMS debugging to Nouveau, along the
> > > > >> style of the previous patches for Intel¹ and Radeon².  I'm only able
> > > > >> to test on nv50 here, so a test on nv04 would be much appreciated,
> > > > >> and I've published instructions on how to test here³.  Thanks!
> > > > >>
> > > > >> - Chris.
> > > > >>     
> > > > >
> > > > > Hi,
> > > > >
> > > > > I just tried that patch, but unfortunately nether with nor without it
> > > > > kdb seems not to work.
> > > > > It could be id10t error from my side, but I did test the kdb in the past
> > > > > with few KMS patches, and it seemed to work.
> > > > >
> > > > > Now I can't even get its prompt on the console.
> > > > >
> > > > > This is what I do:
> > > > >
> > > > > echo kbd | sudo tee /sys/module/kgdboc/parameters/kgdboc
> > > > > (also tried booting with kgdboc=kbd)
> > > > >   
> > > > 
> > > > Try changing it to kgdboc=kms,kbd  or the "echo kms,kbd"
> > > > 
> > > > When you use only the kbd, the kms feature is not activated.
> > > This doesn't help.
> > > 
> > > I am afraid that this bug isn't related to kms, but rather is generic.
> > 
> > I turns out that it was the NMI watchdog that I had enabled.
> > Without it kdb works very well, including the kms support.
> Please disregard this. kdb works with nmi watchdog now as well.
> Probably something was fixed, maybe unrelated to it.
> 
> > 
> > It would be better if you were to detect kms instead of adding an
> > explicit param to kgdboc cmd line.
> > 
> > Also found out that after a debug session with Alt+SysRQ+g and X
> > running, these keys aren't released. I had to press on all of them to
> > make them release.
> > It makes sense as kgdboc in that case reads directly from keyboard port.
> And I see that kgdb actually has a code that works that around.
> I suspect that what happens is that keys are released before X continues
> running, and therefore it doesn't pick these events up.

However an evtest on input event running on kernel tty, still only picks
release of the alt key.

Best regards,
Maxim Levitsky


------------------------------------------------------------------------------
Start uncovering the many advantages of virtual appliances
and start using them to simplify application deployment and
accelerate your shift to cloud computing.
http://p.sf.net/sfu/novell-sfdev2dev
_______________________________________________
Kgdb-bugreport mailing list
Kgdb-bugreport@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport

Re: [Nouveau] [PATCH] drm/nouveau/kms: Implement KDB debug hooks for nouveau KMS.

[Maxim Levitsky]

On Wed, 2010-09-22 at 16:06 +0200, Maxim Levitsky wrote: 
> On Wed, 2010-09-22 at 16:03 +0200, Maxim Levitsky wrote: 
> > On Wed, 2010-09-22 at 03:42 +0200, Maxim Levitsky wrote: 
> > > On Thu, 2010-09-02 at 13:46 +0300, Maxim Levitsky wrote: 
> > > > On Wed, 2010-09-01 at 06:35 -0500, Jason Wessel wrote: 
> > > > > On 09/01/2010 04:56 AM, Maxim Levitsky wrote:
> > > > > > On Thu, 2010-08-19 at 13:55 -0400, Chris Ball wrote: 
> > > > > >   
> > > > > >> Hi,
> > > > > >>
> > > > > >> Here's a patch to add support for KMS debugging to Nouveau, along the
> > > > > >> style of the previous patches for Intel¹ and Radeon².  I'm only able
> > > > > >> to test on nv50 here, so a test on nv04 would be much appreciated,
> > > > > >> and I've published instructions on how to test here³.  Thanks!
> > > > > >>
> > > > > >> - Chris.
> > > > > >>     
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > I just tried that patch, but unfortunately nether with nor without it
> > > > > > kdb seems not to work.
> > > > > > It could be id10t error from my side, but I did test the kdb in the past
> > > > > > with few KMS patches, and it seemed to work.
> > > > > >
> > > > > > Now I can't even get its prompt on the console.
> > > > > >
> > > > > > This is what I do:
> > > > > >
> > > > > > echo kbd | sudo tee /sys/module/kgdboc/parameters/kgdboc
> > > > > > (also tried booting with kgdboc=kbd)
> > > > > >   
> > > > > 
> > > > > Try changing it to kgdboc=kms,kbd  or the "echo kms,kbd"
> > > > > 
> > > > > When you use only the kbd, the kms feature is not activated.
> > > > This doesn't help.
> > > > 
> > > > I am afraid that this bug isn't related to kms, but rather is generic.
> > > 
> > > I turns out that it was the NMI watchdog that I had enabled.
> > > Without it kdb works very well, including the kms support.
> > Please disregard this. kdb works with nmi watchdog now as well.
> > Probably something was fixed, maybe unrelated to it.
> > 
> > > 
> > > It would be better if you were to detect kms instead of adding an
> > > explicit param to kgdboc cmd line.
> > > 
> > > Also found out that after a debug session with Alt+SysRQ+g and X
> > > running, these keys aren't released. I had to press on all of them to
> > > make them release.
> > > It makes sense as kgdboc in that case reads directly from keyboard port.
> > And I see that kgdb actually has a code that works that around.
> > I suspect that what happens is that keys are released before X continues
> > running, and therefore it doesn't pick these events up.
> 
> However an evtest on input event running on kernel tty, still only picks
> release of the alt key.

[Dropped nouveau list, because this is offtopic there]

I pretty much got to the bottom of this.
There are 2 separate issues:


1. SysRq handler is now a input 'filter', which means that it can 'eat'
input events, so they don't show up on input bus.
It does so while sysrq key is down.
So sysrq and 'g' events never reach the kernel kbd driver and therefore
the hack to release them doesn't work.

2. The kbd_clear_keys_helper injects the keyup events alright, but it
doesn't inject SYN events, and therefore X evdev driver doesn't pick
these injected events untill next SYN event. 

This patch makes key release work in expense of showing sysrq key to userspace, which isn't that good,
because now Alt+SysRQ causes a screen capture by default.
In my opinion the sysrq filter should stay.
We should just make kdb hook into atkbd and do the key release there.
This should both result in cleaner/more robust code, and make this issue disappear.
I'll look at doing that.


diff --git a/drivers/char/keyboard.c b/drivers/char/keyboard.c
index 0c6c641..7df6af5 100644
--- a/drivers/char/keyboard.c
+++ b/drivers/char/keyboard.c
@@ -368,6 +368,7 @@ static int kbd_clear_keys_helper(struct input_handle *handle, void *data)
 {
 	unsigned int *keycode = data;
 	input_inject_event(handle, EV_KEY, *keycode, 0);
+	input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
 	return 0;
 }
 
diff --git a/drivers/char/sysrq.c b/drivers/char/sysrq.c
index ef31bb8..db1eb12 100644
--- a/drivers/char/sysrq.c
+++ b/drivers/char/sysrq.c
@@ -601,7 +601,7 @@ static bool sysrq_filter(struct input_handle *handle, unsigned int type,
 	}
 
 out:
-	return sysrq_down;
+	return 0;
 }
 
 static int sysrq_connect(struct input_handler *handler,


Best regards,
	Maxim Levitsky

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Nouveau] [PATCH] drm/nouveau/kms: Implement KDB debug hooks for nouveau KMS.

[Maxim Levitsky]

On Wed, 2010-09-22 at 19:07 +0200, Maxim Levitsky wrote:
> On Wed, 2010-09-22 at 16:06 +0200, Maxim Levitsky wrote: 
> > On Wed, 2010-09-22 at 16:03 +0200, Maxim Levitsky wrote: 
> > > On Wed, 2010-09-22 at 03:42 +0200, Maxim Levitsky wrote: 
> > > > On Thu, 2010-09-02 at 13:46 +0300, Maxim Levitsky wrote: 
> > > > > On Wed, 2010-09-01 at 06:35 -0500, Jason Wessel wrote: 
> > > > > > On 09/01/2010 04:56 AM, Maxim Levitsky wrote:
> > > > > > > On Thu, 2010-08-19 at 13:55 -0400, Chris Ball wrote: 
> > > > > > >   
> > > > > > >> Hi,
> > > > > > >>
> > > > > > >> Here's a patch to add support for KMS debugging to Nouveau, along the
> > > > > > >> style of the previous patches for Intel¹ and Radeon².  I'm only able
> > > > > > >> to test on nv50 here, so a test on nv04 would be much appreciated,
> > > > > > >> and I've published instructions on how to test here³.  Thanks!
> > > > > > >>
> > > > > > >> - Chris.
> > > > > > >>     
> > > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > I just tried that patch, but unfortunately nether with nor without it
> > > > > > > kdb seems not to work.
> > > > > > > It could be id10t error from my side, but I did test the kdb in the past
> > > > > > > with few KMS patches, and it seemed to work.
> > > > > > >
> > > > > > > Now I can't even get its prompt on the console.
> > > > > > >
> > > > > > > This is what I do:
> > > > > > >
> > > > > > > echo kbd | sudo tee /sys/module/kgdboc/parameters/kgdboc
> > > > > > > (also tried booting with kgdboc=kbd)
> > > > > > >   
> > > > > > 
> > > > > > Try changing it to kgdboc=kms,kbd  or the "echo kms,kbd"
> > > > > > 
> > > > > > When you use only the kbd, the kms feature is not activated.
> > > > > This doesn't help.
> > > > > 
> > > > > I am afraid that this bug isn't related to kms, but rather is generic.
> > > > 
> > > > I turns out that it was the NMI watchdog that I had enabled.
> > > > Without it kdb works very well, including the kms support.
> > > Please disregard this. kdb works with nmi watchdog now as well.
> > > Probably something was fixed, maybe unrelated to it.
> > > 
> > > > 
> > > > It would be better if you were to detect kms instead of adding an
> > > > explicit param to kgdboc cmd line.
> > > > 
> > > > Also found out that after a debug session with Alt+SysRQ+g and X
> > > > running, these keys aren't released. I had to press on all of them to
> > > > make them release.
> > > > It makes sense as kgdboc in that case reads directly from keyboard port.
> > > And I see that kgdb actually has a code that works that around.
> > > I suspect that what happens is that keys are released before X continues
> > > running, and therefore it doesn't pick these events up.
> > 
> > However an evtest on input event running on kernel tty, still only picks
> > release of the alt key.
> 
> [Dropped nouveau list, because this is offtopic there]
> 
> I pretty much got to the bottom of this.
> There are 2 separate issues:
> 
> 
> 1. SysRq handler is now a input 'filter', which means that it can 'eat'
> input events, so they don't show up on input bus.
> It does so while sysrq key is down.
> So sysrq and 'g' events never reach the kernel kbd driver and therefore
> the hack to release them doesn't work.
> 
> 2. The kbd_clear_keys_helper injects the keyup events alright, but it
> doesn't inject SYN events, and therefore X evdev driver doesn't pick
> these injected events untill next SYN event. 
> 
> This patch makes key release work in expense of showing sysrq key to userspace, which isn't that good,
> because now Alt+SysRQ causes a screen capture by default.
> In my opinion the sysrq filter should stay.
> We should just make kdb hook into atkbd and do the key release there.
> This should both result in cleaner/more robust code, and make this issue disappear.
> I'll look at doing that.
> 
> 
> diff --git a/drivers/char/keyboard.c b/drivers/char/keyboard.c
> index 0c6c641..7df6af5 100644
> --- a/drivers/char/keyboard.c
> +++ b/drivers/char/keyboard.c
> @@ -368,6 +368,7 @@ static int kbd_clear_keys_helper(struct input_handle *handle, void *data)
>  {
>  	unsigned int *keycode = data;
>  	input_inject_event(handle, EV_KEY, *keycode, 0);
> +	input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
>  	return 0;
>  }
>  
> diff --git a/drivers/char/sysrq.c b/drivers/char/sysrq.c
> index ef31bb8..db1eb12 100644
> --- a/drivers/char/sysrq.c
> +++ b/drivers/char/sysrq.c
> @@ -601,7 +601,7 @@ static bool sysrq_filter(struct input_handle *handle, unsigned int type,
>  	}
>  
>  out:
> -	return sysrq_down;
> +	return 0;
>  }
>  
>  static int sysrq_connect(struct input_handler *handler,
> 
> 
> Best regards,
> 	Maxim Levitsky
> 
Ping.

Maxim Levitsky


------------------------------------------------------------------------------
Start uncovering the many advantages of virtual appliances
and start using them to simplify application deployment and
accelerate your shift to cloud computing.
http://p.sf.net/sfu/novell-sfdev2dev
_______________________________________________
Kgdb-bugreport mailing list
Kgdb-bugreport@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport

Re: [Nouveau] [PATCH] drm/nouveau/kms: Implement KDB debug hooks for nouveau KMS.

[Jason Wessel]

On 09/24/2010 03:50 PM, Maxim Levitsky wrote:
>   
>> [Dropped nouveau list, because this is offtopic there]
>>
>> I pretty much got to the bottom of this.
>> There are 2 separate issues:
>>
>>
>> 1. SysRq handler is now a input 'filter', which means that it can 'eat'
>> input events, so they don't show up on input bus.
>> It does so while sysrq key is down.
>> So sysrq and 'g' events never reach the kernel kbd driver and therefore
>> the hack to release them doesn't work.
>>
>> 2. The kbd_clear_keys_helper injects the keyup events alright, but it
>> doesn't inject SYN events, and therefore X evdev driver doesn't pick
>> these injected events untill next SYN event. 
>>
>> This patch makes key release work in expense of showing sysrq key to userspace, which isn't that good,
>> because now Alt+SysRQ causes a screen capture by default.
>> In my opinion the sysrq filter should stay.
>> We should just make kdb hook into atkbd and do the key release there.
>> This should both result in cleaner/more robust code, and make this issue disappear.
>> I'll look at doing that.
>>
>>
>> diff --git a/drivers/char/keyboard.c b/drivers/char/keyboard.c
>> index 0c6c641..7df6af5 100644
>> --- a/drivers/char/keyboard.c
>> +++ b/drivers/char/keyboard.c
>> @@ -368,6 +368,7 @@ static int kbd_clear_keys_helper(struct input_handle *handle, void *data)
>>  {
>>  	unsigned int *keycode = data;
>>  	input_inject_event(handle, EV_KEY, *keycode, 0);
>> +	input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
>>  	return 0;
>>  }
>>  
>> diff --git a/drivers/char/sysrq.c b/drivers/char/sysrq.c
>> index ef31bb8..db1eb12 100644
>> --- a/drivers/char/sysrq.c
>> +++ b/drivers/char/sysrq.c
>> @@ -601,7 +601,7 @@ static bool sysrq_filter(struct input_handle *handle, unsigned int type,
>>  	}
>>  
>>  out:
>> -	return sysrq_down;
>> +	return 0;
>>  }
>>  
>>  static int sysrq_connect(struct input_handler *handler,
>>
>>
>> Best regards,
>> 	Maxim Levitsky
>>
>>     

Hi Maxim,

Is it the case that this patch takes care of masking the sysrq event
from user space?

Originally before it became an input filter I had a patch to the
keyboard.c to completely consume the keypress of the sysrq-g.

I have been less than successful at getting the keyboard changes
upstreamed, and I was probably going to ping AKPM, because on that front
I had not received any kind of response either (linux-input that is).

Certainly I can carry your patch in my branch until we can determine a
final upstream design.

Thanks,
Jason.

------------------------------------------------------------------------------
Start uncovering the many advantages of virtual appliances
and start using them to simplify application deployment and
accelerate your shift to cloud computing.
http://p.sf.net/sfu/novell-sfdev2dev

Re: [Nouveau] [PATCH] drm/nouveau/kms: Implement KDB debug hooks for nouveau KMS.

[Maxim Levitsky]

On Fri, 2010-09-24 at 15:58 -0500, Jason Wessel wrote:
> On 09/24/2010 03:50 PM, Maxim Levitsky wrote:
> >   
> >> [Dropped nouveau list, because this is offtopic there]
> >>
> >> I pretty much got to the bottom of this.
> >> There are 2 separate issues:
> >>
> >>
> >> 1. SysRq handler is now a input 'filter', which means that it can 'eat'
> >> input events, so they don't show up on input bus.
> >> It does so while sysrq key is down.
> >> So sysrq and 'g' events never reach the kernel kbd driver and therefore
> >> the hack to release them doesn't work.
> >>
> >> 2. The kbd_clear_keys_helper injects the keyup events alright, but it
> >> doesn't inject SYN events, and therefore X evdev driver doesn't pick
> >> these injected events untill next SYN event. 
> >>
> >> This patch makes key release work in expense of showing sysrq key to userspace, which isn't that good,
> >> because now Alt+SysRQ causes a screen capture by default.
> >> In my opinion the sysrq filter should stay.
> >> We should just make kdb hook into atkbd and do the key release there.
> >> This should both result in cleaner/more robust code, and make this issue disappear.
> >> I'll look at doing that.
> >>
> >>
> >> diff --git a/drivers/char/keyboard.c b/drivers/char/keyboard.c
> >> index 0c6c641..7df6af5 100644
> >> --- a/drivers/char/keyboard.c
> >> +++ b/drivers/char/keyboard.c
> >> @@ -368,6 +368,7 @@ static int kbd_clear_keys_helper(struct input_handle *handle, void *data)
> >>  {
> >>  	unsigned int *keycode = data;
> >>  	input_inject_event(handle, EV_KEY, *keycode, 0);
> >> +	input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
> >>  	return 0;
> >>  }
> >>  
> >> diff --git a/drivers/char/sysrq.c b/drivers/char/sysrq.c
> >> index ef31bb8..db1eb12 100644
> >> --- a/drivers/char/sysrq.c
> >> +++ b/drivers/char/sysrq.c
> >> @@ -601,7 +601,7 @@ static bool sysrq_filter(struct input_handle *handle, unsigned int type,
> >>  	}
> >>  
> >>  out:
> >> -	return sysrq_down;
> >> +	return 0;
> >>  }
> >>  
> >>  static int sysrq_connect(struct input_handler *handler,
> >>
> >>
> >> Best regards,
> >> 	Maxim Levitsky
> >>
> >>     
> 
> Hi Maxim,
> 
> Is it the case that this patch takes care of masking the sysrq event
> from user space?
> 
> Originally before it became an input filter I had a patch to the
> keyboard.c to completely consume the keypress of the sysrq-g.
> 
> I have been less than successful at getting the keyboard changes
> upstreamed, and I was probably going to ping AKPM, because on that front
> I had not received any kind of response either (linux-input that is).
> 
> Certainly I can carry your patch in my branch until we can determine a
> final upstream design.

Its an option, but bear in the mind that my 'patch' causes very
unpleasant regression.
The regression is that now Alt+SysRQ is reported to userspace and
treated as a PrintScreen which in gnome is treated as a launcher for
screenshot application.
So after you pressed it, you will end up with an army of
gnome-screenshots poping up.
So I think that tty kbd driver is wrong place for key release.

The right solution in my opinion is to make atkbd register with kgdb and
provide to it, the polling keyboard IO services, and also take care of
releasing the keys as soon as debug mode is entered or exited.

Btw, can a driver register a hook into kgdb to be executed on
enter/leave of the debug mode?
If so the atkbd driver should just do that.

I am also aware of the fact that atkbd doesn't talk to hardware
directly.
Thus 'the' proper fix is also to add a polled serio handlers, make atkbd
use them
if available and if so, register with the kgdb.

Best regards,
	Maxim Levitsky


------------------------------------------------------------------------------
Start uncovering the many advantages of virtual appliances
and start using them to simplify application deployment and
accelerate your shift to cloud computing.
http://p.sf.net/sfu/novell-sfdev2dev

Re: [Nouveau] [PATCH] drm/nouveau/kms: Implement KDB debug hooks for nouveau KMS.

[Dmitry Torokhov]

On Sat, Sep 25, 2010 at 02:14:32AM +0200, Maxim Levitsky wrote:
> On Fri, 2010-09-24 at 15:58 -0500, Jason Wessel wrote:
> > On 09/24/2010 03:50 PM, Maxim Levitsky wrote:
> > >   
> > >> [Dropped nouveau list, because this is offtopic there]
> > >>
> > >> I pretty much got to the bottom of this.
> > >> There are 2 separate issues:
> > >>
> > >>
> > >> 1. SysRq handler is now a input 'filter', which means that it can 'eat'
> > >> input events, so they don't show up on input bus.
> > >> It does so while sysrq key is down.
> > >> So sysrq and 'g' events never reach the kernel kbd driver and therefore
> > >> the hack to release them doesn't work.
> > >>
> > >> 2. The kbd_clear_keys_helper injects the keyup events alright, but it
> > >> doesn't inject SYN events, and therefore X evdev driver doesn't pick
> > >> these injected events untill next SYN event. 
> > >>
> > >> This patch makes key release work in expense of showing sysrq key to userspace, which isn't that good,
> > >> because now Alt+SysRQ causes a screen capture by default.
> > >> In my opinion the sysrq filter should stay.
> > >> We should just make kdb hook into atkbd and do the key release there.
> > >> This should both result in cleaner/more robust code, and make this issue disappear.
> > >> I'll look at doing that.
> > >>
> > >>
> > >> diff --git a/drivers/char/keyboard.c b/drivers/char/keyboard.c
> > >> index 0c6c641..7df6af5 100644
> > >> --- a/drivers/char/keyboard.c
> > >> +++ b/drivers/char/keyboard.c
> > >> @@ -368,6 +368,7 @@ static int kbd_clear_keys_helper(struct input_handle *handle, void *data)
> > >>  {
> > >>  	unsigned int *keycode = data;
> > >>  	input_inject_event(handle, EV_KEY, *keycode, 0);
> > >> +	input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
> > >>  	return 0;
> > >>  }
> > >>  
> > >> diff --git a/drivers/char/sysrq.c b/drivers/char/sysrq.c
> > >> index ef31bb8..db1eb12 100644
> > >> --- a/drivers/char/sysrq.c
> > >> +++ b/drivers/char/sysrq.c
> > >> @@ -601,7 +601,7 @@ static bool sysrq_filter(struct input_handle *handle, unsigned int type,
> > >>  	}
> > >>  
> > >>  out:
> > >> -	return sysrq_down;
> > >> +	return 0;
> > >>  }
> > >>  
> > >>  static int sysrq_connect(struct input_handler *handler,
> > >>
> > >>
> > >> Best regards,
> > >> 	Maxim Levitsky
> > >>
> > >>     
> > 
> > Hi Maxim,
> > 
> > Is it the case that this patch takes care of masking the sysrq event
> > from user space?
> > 
> > Originally before it became an input filter I had a patch to the
> > keyboard.c to completely consume the keypress of the sysrq-g.
> > 
> > I have been less than successful at getting the keyboard changes
> > upstreamed, and I was probably going to ping AKPM, because on that front
> > I had not received any kind of response either (linux-input that is).
> > 
> > Certainly I can carry your patch in my branch until we can determine a
> > final upstream design.
> 
> Its an option, but bear in the mind that my 'patch' causes very
> unpleasant regression.
> The regression is that now Alt+SysRQ is reported to userspace and
> treated as a PrintScreen which in gnome is treated as a launcher for
> screenshot application.
> So after you pressed it, you will end up with an army of
> gnome-screenshots poping up.
> So I think that tty kbd driver is wrong place for key release.
> 

Agree.

> The right solution in my opinion is to make atkbd register with kgdb and
> provide to it, the polling keyboard IO services, and also take care of
> releasing the keys as soon as debug mode is entered or exited.

Disagree. We may support different keyboard devices with kdb or use one
keyboard (USB) to drop into debugger and then switch to atkbd...

I think we need to move Jason's code from drivers/char/keyboard.c to
drivers/char/sysrq.c and have it track keys that have been kept pressed
before entering sysrq mode and release them when leaving the mode.

We also need to teach sysrq that some handlers need resetting of the
keyboard state.

> 
> Btw, can a driver register a hook into kgdb to be executed on
> enter/leave of the debug mode?
> If so the atkbd driver should just do that.
> 
> I am also aware of the fact that atkbd doesn't talk to hardware
> directly.
> Thus 'the' proper fix is also to add a polled serio handlers, make atkbd
> use them
> if available and if so, register with the kgdb.

The issue is not that interrupts are not available but that we are
holding too many locks...

-- 
Dmitry

Re: [Nouveau] [PATCH] drm/nouveau/kms: Implement KDB debug hooks for nouveau KMS.

[Jason Wessel]

[-- Attachment #1: Type: text/plain, Size: 3749 bytes --]

On 09/24/2010 11:08 PM, Dmitry Torokhov wrote:
> On Sat, Sep 25, 2010 at 02:14:32AM +0200, Maxim Levitsky wrote:
>   
>> On Fri, 2010-09-24 at 15:58 -0500, Jason Wessel wrote:
>>     
>>> On 09/24/2010 03:50 PM, Maxim Levitsky wrote:
>>>       
>>>>   
>>>>         
>>>>> [Dropped nouveau list, because this is offtopic there]
>>>>>
>>>>> I pretty much got to the bottom of this.
>>>>> There are 2 separate issues:
>>>>>
>>>>>
>>>>> 1. SysRq handler is now a input 'filter', which means that it can 'eat'
>>>>> input events, so they don't show up on input bus.
>>>>> It does so while sysrq key is down.
>>>>> So sysrq and 'g' events never reach the kernel kbd driver and therefore
>>>>> the hack to release them doesn't work.
>>>>>
>>>>> 2. The kbd_clear_keys_helper injects the keyup events alright, but it
>>>>> doesn't inject SYN events, and therefore X evdev driver doesn't pick
>>>>> these injected events untill next SYN event. 
>>>>>
>>>>> This patch makes key release work in expense of showing sysrq key to userspace, which isn't that good,
>>>>> because now Alt+SysRQ causes a screen capture by default.
>>>>> In my opinion the sysrq filter should stay.
>>>>> We should just make kdb hook into atkbd and do the key release there.
>>>>> This should both result in cleaner/more robust code, and make this issue disappear.
>>>>> I'll look at doing that.
>>>>>
>>>>>
>>>>> diff --git a/drivers/char/keyboard.c b/drivers/char/keyboard.c
>>>>> index 0c6c641..7df6af5 100644
>>>>> --- a/drivers/char/keyboard.c
>>>>> +++ b/drivers/char/keyboard.c
>>>>> @@ -368,6 +368,7 @@ static int kbd_clear_keys_helper(struct input_handle *handle, void *data)
>>>>>  {
>>>>>  	unsigned int *keycode = data;
>>>>>  	input_inject_event(handle, EV_KEY, *keycode, 0);
>>>>> +	input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
>>>>>  	return 0;
>>>>>  }
>>>>>           

So I kept this piece, in the 0002 patch which is attached.   I revised
the keyboard/input series at:

http://git.kernel.org/?p=linux/kernel/git/jwessel/linux-2.6-kgdb.git;a=shortlog;h=refs/heads/for_input

>>>>>  
>>>>> diff --git a/drivers/char/sysrq.c b/drivers/char/sysrq.c
>>>>> index ef31bb8..db1eb12 100644
>>>>> --- a/drivers/char/sysrq.c
>>>>> +++ b/drivers/char/sysrq.c
>>>>> @@ -601,7 +601,7 @@ static bool sysrq_filter(struct input_handle *handle, unsigned int type,
>>>>>  	}
>>>>>  
>>>>>  out:
>>>>> -	return sysrq_down;
>>>>> +	return 0;
>>>>>  }
>>>>>           

I took some time to look more closely at the problem, and I believe we
still want to filter out all these input events.

>   
>> The right solution in my opinion is to make atkbd register with kgdb and
>> provide to it, the polling keyboard IO services, and also take care of
>> releasing the keys as soon as debug mode is entered or exited.
>>     
>
> Disagree. We may support different keyboard devices with kdb or use one
> keyboard (USB) to drop into debugger and then switch to atkbd...
>
> I think we need to move Jason's code from drivers/char/keyboard.c to
> drivers/char/sysrq.c and have it track keys that have been kept pressed
> before entering sysrq mode and release them when leaving the mode.
>
> We also need to teach sysrq that some handlers need resetting of the
> keyboard state.
>
>   

I found the root of the problem was the low level keyboard bit mask
getting out of sync in the input layer.   Perhaps Dmitry can have a look
at the 3rd patch in the series, which addresses the problem.  

I also put these patches into kgdb-next.

I believe it fixes the problems Maxim encountered, or at least the
problems I observed independently with stuck keys and the lack of the
ability to correctly use alt-PrintScreen.

If you are willing Maxim, can you give these patches a go?

Thanks,
Jason.

[-- Attachment #2: 0001-keyboard-kgdboc-Allow-key-release-on-kernel-resume.patch --]
[-- Type: text/x-diff, Size: 4152 bytes --]

>From ed6fb201ca864e977c6a4fcf345014fd1d4ebed4 Mon Sep 17 00:00:00 2001
From: Jason Wessel <jason.wessel@windriver.com>
Date: Sun, 26 Sep 2010 06:33:36 -0500
Subject: [PATCH 1/3] keyboard,kgdboc: Allow key release on kernel resume

When using a keyboard with kdb, a hook point to free all the
keystrokes is required for resuming kernel operations.

This is mainly because there is no way to force the end user to hold
down the original keys that were pressed prior to entering kdb when
resuming the kernel.

The kgdboc driver will call kbd_dbg_clear_keys() just prior to
resuming the kernel execution which will schedule a callback to clear
any keys which were depressed prior to the entering the kernel
debugger.

CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
CC: Greg Kroah-Hartman <gregkh@suse.de>
CC: linux-input@vger.kernel.org
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 drivers/char/keyboard.c  |   37 +++++++++++++++++++++++++++++++++++++
 drivers/serial/kgdboc.c  |   13 +++++++++++++
 include/linux/kbd_kern.h |    1 +
 3 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/drivers/char/keyboard.c b/drivers/char/keyboard.c
index a7ca752..0c6c641 100644
--- a/drivers/char/keyboard.c
+++ b/drivers/char/keyboard.c
@@ -363,6 +363,43 @@ static void to_utf8(struct vc_data *vc, uint c)
 	}
 }
 
+#ifdef CONFIG_KDB_KEYBOARD
+static int kbd_clear_keys_helper(struct input_handle *handle, void *data)
+{
+	unsigned int *keycode = data;
+	input_inject_event(handle, EV_KEY, *keycode, 0);
+	return 0;
+}
+
+static void kbd_clear_keys_callback(struct work_struct *dummy)
+{
+	unsigned int i, j, k;
+
+	for (i = 0; i < ARRAY_SIZE(key_down); i++) {
+		if (!key_down[i])
+			continue;
+
+		k = i * BITS_PER_LONG;
+
+		for (j = 0; j < BITS_PER_LONG; j++, k++) {
+			if (!test_bit(k, key_down))
+				continue;
+			input_handler_for_each_handle(&kbd_handler, &k,
+				      kbd_clear_keys_helper);
+		}
+	}
+}
+
+static DECLARE_WORK(kbd_clear_keys_work, kbd_clear_keys_callback);
+
+/* Called to clear any key presses after resuming the kernel. */
+void kbd_dbg_clear_keys(void)
+{
+	schedule_work(&kbd_clear_keys_work);
+}
+EXPORT_SYMBOL_GPL(kbd_dbg_clear_keys);
+#endif /* CONFIG_KDB_KEYBOARD */
+
 /*
  * Called after returning from RAW mode or when changing consoles - recompute
  * shift_down[] and shift_state from key_down[] maybe called when keymap is
diff --git a/drivers/serial/kgdboc.c b/drivers/serial/kgdboc.c
index 39f9a1a..62b6edc 100644
--- a/drivers/serial/kgdboc.c
+++ b/drivers/serial/kgdboc.c
@@ -18,6 +18,7 @@
 #include <linux/tty.h>
 #include <linux/console.h>
 #include <linux/vt_kern.h>
+#include <linux/kbd_kern.h>
 
 #define MAX_CONFIG_LEN		40
 
@@ -37,12 +38,16 @@ static struct tty_driver	*kgdb_tty_driver;
 static int			kgdb_tty_line;
 
 #ifdef CONFIG_KDB_KEYBOARD
+static bool			kgdboc_use_kbd;
+
 static int kgdboc_register_kbd(char **cptr)
 {
+	kgdboc_use_kbd = false;
 	if (strncmp(*cptr, "kbd", 3) == 0) {
 		if (kdb_poll_idx < KDB_POLL_FUNC_MAX) {
 			kdb_poll_funcs[kdb_poll_idx] = kdb_get_kbd_char;
 			kdb_poll_idx++;
+			kgdboc_use_kbd = true;
 			if (cptr[0][3] == ',')
 				*cptr += 4;
 			else
@@ -65,9 +70,16 @@ static void kgdboc_unregister_kbd(void)
 		}
 	}
 }
+
+static inline void kgdboc_clear_kbd(void)
+{
+	if (kgdboc_use_kbd)
+		kbd_dbg_clear_keys(); /* Release all pressed keys */
+}
 #else /* ! CONFIG_KDB_KEYBOARD */
 #define kgdboc_register_kbd(x) 0
 #define kgdboc_unregister_kbd()
+#define kgdboc_clear_kbd()
 #endif /* ! CONFIG_KDB_KEYBOARD */
 
 static int kgdboc_option_setup(char *opt)
@@ -231,6 +243,7 @@ static void kgdboc_post_exp_handler(void)
 		dbg_restore_graphics = 0;
 		con_debug_leave();
 	}
+	kgdboc_clear_kbd();
 }
 
 static struct kgdb_io kgdboc_io_ops = {
diff --git a/include/linux/kbd_kern.h b/include/linux/kbd_kern.h
index 506ad20..ae87c0a 100644
--- a/include/linux/kbd_kern.h
+++ b/include/linux/kbd_kern.h
@@ -144,6 +144,7 @@ struct console;
 int getkeycode(unsigned int scancode);
 int setkeycode(unsigned int scancode, unsigned int keycode);
 void compute_shiftstate(void);
+void kbd_dbg_clear_keys(void);
 
 /* defkeymap.c */
 
-- 
1.6.3.3


[-- Attachment #3: 0002-keyboard-kdb-inject-SYN-events-in-kbd_clear_keys_hel.patch --]
[-- Type: text/x-diff, Size: 1036 bytes --]

>From 43eb157cdf4213e8b7792746e7d11afec2309205 Mon Sep 17 00:00:00 2001
From: Maxim Levitsky <maximlevitsky@gmail.com>
Date: Wed, 22 Sep 2010 10:07:05 -0700
Subject: [PATCH 2/3] keyboard,kdb: inject SYN events in kbd_clear_keys_helper

The kbd_clear_keys_helper injects the keyup events alright, but it
doesn't inject SYN events, and therefore X evdev driver doesn't pick
these injected events untill next SYN event.

Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 drivers/char/keyboard.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/char/keyboard.c b/drivers/char/keyboard.c
index 0c6c641..7df6af5 100644
--- a/drivers/char/keyboard.c
+++ b/drivers/char/keyboard.c
@@ -368,6 +368,7 @@ static int kbd_clear_keys_helper(struct input_handle *handle, void *data)
 {
 	unsigned int *keycode = data;
 	input_inject_event(handle, EV_KEY, *keycode, 0);
+	input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
 	return 0;
 }
 
-- 
1.6.3.3


[-- Attachment #4: 0003-sysrq-keyboard-properly-deal-with-alt-sysrq-in-sysrq.patch --]
[-- Type: text/x-diff, Size: 3892 bytes --]

>From 33dea0e730e03814f0cd71c604185ba1c7fad51d Mon Sep 17 00:00:00 2001
From: Jason Wessel <jason.wessel@windriver.com>
Date: Tue, 5 Oct 2010 14:38:36 -0500
Subject: [PATCH 3/3] sysrq,keyboard: properly deal with alt-sysrq in sysrq input filter

This patch addresses 2 problems:

1) You should still be able to use alt-PrintScreen to capture a grab
   of a single window when the sysrq filter is active.

2) The sysrq filter should reset the low level key mask so that future
   key presses will not show up as a repeated key.  The problem was
   that when you executed alt-sysrq g and then resumed the kernel, you
   would have to press 'g' twice to get it working again.

CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
CC: Greg Kroah-Hartman <gregkh@suse.de>
CC: linux-input@vger.kernel.org
Reported-by: Maxim Levitsky <maximlevitsky@gmail.com>
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 drivers/char/sysrq.c |   64 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/drivers/char/sysrq.c b/drivers/char/sysrq.c
index ef31bb8..9b97aad 100644
--- a/drivers/char/sysrq.c
+++ b/drivers/char/sysrq.c
@@ -566,6 +566,57 @@ static const unsigned char sysrq_xlate[KEY_MAX + 1] =
 static bool sysrq_down;
 static int sysrq_alt_use;
 static int sysrq_alt;
+static bool sysrq_kbd_triggered;
+
+/*
+ * This function was a copy of input_pass_event but modified to allow
+ * by-passing a specific filter, to allow for injected events without
+ * filter recursion.
+ */
+static void input_pass_event_ignore(struct input_dev *dev,
+			     unsigned int type, unsigned int code, int value,
+			     struct input_handle *ignore_handle)
+{
+	struct input_handler *handler;
+	struct input_handle *handle;
+
+	rcu_read_lock();
+
+	handle = rcu_dereference(dev->grab);
+	if (handle)
+		handle->handler->event(handle, type, code, value);
+	else {
+		bool filtered = false;
+
+		list_for_each_entry_rcu(handle, &dev->h_list, d_node) {
+			if (!handle->open || handle == ignore_handle)
+				continue;
+			handler = handle->handler;
+			if (!handler->filter) {
+				if (filtered)
+					break;
+
+				handler->event(handle, type, code, value);
+
+			} else if (handler->filter(handle, type, code, value))
+				filtered = true;
+		}
+	}
+
+	rcu_read_unlock();
+}
+
+/*
+ * Pass along alt-print_screen, if there was no sysrq processing by
+ * sending a key press down and then passing the key up event.
+ */
+static void simulate_alt_sysrq(struct input_handle *handle)
+{
+	input_pass_event_ignore(handle->dev, EV_KEY, KEY_SYSRQ, 1, handle);
+	input_pass_event_ignore(handle->dev, EV_SYN, SYN_REPORT, 0, handle);
+	input_pass_event_ignore(handle->dev, EV_KEY, KEY_SYSRQ, 0, handle);
+	input_pass_event_ignore(handle->dev, EV_SYN, SYN_REPORT, 0, handle);
+}
 
 static bool sysrq_filter(struct input_handle *handle, unsigned int type,
 		         unsigned int code, int value)
@@ -580,9 +631,11 @@ static bool sysrq_filter(struct input_handle *handle, unsigned int type,
 		if (value)
 			sysrq_alt = code;
 		else {
-			if (sysrq_down && code == sysrq_alt_use)
+			if (sysrq_down && code == sysrq_alt_use) {
 				sysrq_down = false;
-
+				if (!sysrq_kbd_triggered)
+					simulate_alt_sysrq(handle);
+			}
 			sysrq_alt = 0;
 		}
 		break;
@@ -590,13 +643,18 @@ static bool sysrq_filter(struct input_handle *handle, unsigned int type,
 	case KEY_SYSRQ:
 		if (value == 1 && sysrq_alt) {
 			sysrq_down = true;
+			sysrq_kbd_triggered = false;
 			sysrq_alt_use = sysrq_alt;
 		}
 		break;
 
 	default:
-		if (sysrq_down && value && value != 2)
+		if (sysrq_down && value && value != 2 && !sysrq_kbd_triggered) {
+			sysrq_kbd_triggered = true;
 			__handle_sysrq(sysrq_xlate[code], true);
+			/* Clear any handled keys from being flagged as a repeated stroke */
+			__clear_bit(code, handle->dev->key);
+		}
 		break;
 	}
 
-- 
1.6.3.3

Re: [Nouveau] [PATCH] drm/nouveau/kms: Implement KDB debug hooks for nouveau KMS.

[Maxim Levitsky]

On Tue, 2010-10-05 at 15:56 -0500, Jason Wessel wrote:
> On 09/24/2010 11:08 PM, Dmitry Torokhov wrote:
> > On Sat, Sep 25, 2010 at 02:14:32AM +0200, Maxim Levitsky wrote:
> >   
> >> On Fri, 2010-09-24 at 15:58 -0500, Jason Wessel wrote:
> >>     
> >>> On 09/24/2010 03:50 PM, Maxim Levitsky wrote:
> >>>       
> >>>>   
> >>>>         
> >>>>> [Dropped nouveau list, because this is offtopic there]
> >>>>>
> >>>>> I pretty much got to the bottom of this.
> >>>>> There are 2 separate issues:
> >>>>>
> >>>>>
> >>>>> 1. SysRq handler is now a input 'filter', which means that it can 'eat'
> >>>>> input events, so they don't show up on input bus.
> >>>>> It does so while sysrq key is down.
> >>>>> So sysrq and 'g' events never reach the kernel kbd driver and therefore
> >>>>> the hack to release them doesn't work.
> >>>>>
> >>>>> 2. The kbd_clear_keys_helper injects the keyup events alright, but it
> >>>>> doesn't inject SYN events, and therefore X evdev driver doesn't pick
> >>>>> these injected events untill next SYN event. 
> >>>>>
> >>>>> This patch makes key release work in expense of showing sysrq key to userspace, which isn't that good,
> >>>>> because now Alt+SysRQ causes a screen capture by default.
> >>>>> In my opinion the sysrq filter should stay.
> >>>>> We should just make kdb hook into atkbd and do the key release there.
> >>>>> This should both result in cleaner/more robust code, and make this issue disappear.
> >>>>> I'll look at doing that.
> >>>>>
Nope, still same problem.
Maybe more keys were released, but still
pressing Alt+SysRQ+g second time doesn't break to the debugger.

I pulled your kgdb-next branch which does contain these patches.

Best regards,
	Maxim Levitsky


------------------------------------------------------------------------------
Beautiful is writing same markup. Internet Explorer 9 supports
standards for HTML5, CSS3, SVG 1.1,  ECMAScript5, and DOM L2 & L3.
Spend less time writing and  rewriting code and more time creating great
experiences on the web. Be a part of the beta today.
http://p.sf.net/sfu/beautyoftheweb

sysrq filter and stuck keys [ was Re: [Nouveau] [PATCH] drm/nouveau/kms: Implement KDB debug hooks for nouveau KMS.]

[Jason Wessel]

On 10/13/2010 09:34 PM, Maxim Levitsky wrote:
>
> Nope, still same problem.
> Maybe more keys were released, but still
> pressing Alt+SysRQ+g second time doesn't break to the debugger.
>
>   

Thanks for trying this out Maxim.

I have to wonder if part of the problem is the way that raw events can
propagate directly via the old keyboard interface vs the input layer.  
I had noticed something somewhat similar with an xorg.conf which
appeared to be using both the raw keyboard and the input layer for
input.  I was having all sorts of problems until I changed the xorg.conf
settings to use the input layer.  I noticed that even pressing the up
arrow in an xterm was invoking the screen shot mechanism for example.

Perhaps you can provide some more data to see if this is a similar
problem or not.

On the Fedora 13 test system I made sure the following were commented
out in the xorg.conf file.

#       InputDevice    "Keyboard0" "CoreKeyboard"

#       Option "AllowEmptyInput" "off"


This had resolved the issue at least for the system with the Intel
graphics chip and non-usb keyboard.

The next step might be to have you run with an instrumented kernel if
this is not the issue, and of course if you are willing to help further
debug this.

Thanks,
Jason.

Re: sysrq filter and stuck keys [ was Re: [Nouveau] [PATCH] drm/nouveau/kms: Implement KDB debug hooks for nouveau KMS.]

[Maxim Levitsky]

On Wed, 2010-10-20 at 11:01 -0500, Jason Wessel wrote:
> On 10/13/2010 09:34 PM, Maxim Levitsky wrote:
> >
> > Nope, still same problem.
> > Maybe more keys were released, but still
> > pressing Alt+SysRQ+g second time doesn't break to the debugger.
> >
> >   
> 
> Thanks for trying this out Maxim.
> 
> I have to wonder if part of the problem is the way that raw events can
> propagate directly via the old keyboard interface vs the input layer.  
> I had noticed something somewhat similar with an xorg.conf which
> appeared to be using both the raw keyboard and the input layer for
> input.  I was having all sorts of problems until I changed the xorg.conf
> settings to use the input layer.  I noticed that even pressing the up
> arrow in an xterm was invoking the screen shot mechanism for example.
> 
> Perhaps you can provide some more data to see if this is a similar
> problem or not.

Jardon, I already found the root cause of the problem.
First of all, note that I use xorg's evdev driver.
The in-kernel tty kbd driver is only used for virtual consoles.

The root cause is that in-kernel kbd driver beeing a client of the input
subsystem just doesn't see the sysrq keys because they are filtered,
therefore it can't release them.

The right solution is to somehow hook at the atkbd driver , tell it that
kbd tampered with its hardware, and make it release the keys.

But that is easy to say, not that easy to do...

the in-kernel kbd driver just isn't the right place for the job.

Best regards,
	Maxim Levitsky

Re: sysrq filter and stuck keys [ was Re: [Nouveau] [PATCH] drm/nouveau/kms: Implement KDB debug hooks for nouveau KMS.]

[Jason Wessel]

On 10/22/2010 09:29 PM, Maxim Levitsky wrote:
> Jason, I already found the root cause of the problem.
> First of all, note that I use xorg's evdev driver.
> The in-kernel tty kbd driver is only used for virtual consoles.
>
>   

I would like to clarify where you observed the problem.  Was it only on
the tty console in X or both places?

> The root cause is that in-kernel kbd driver beeing a client of the input
> subsystem just doesn't see the sysrq keys because they are filtered,
> therefore it can't release them.
>
>   

With the 3 patches to the input layer in kgdb-next I cannot duplicate
the problem at all.  If I remove the last patch in the series, and use
the tty console, I can definitely see the behavior you mention.  I had
added a change to the sysrq input filter (which is consumes the key
strokes) that also resets the key mask in the tty keyboard driver

Specifically in

+                       /* Clear handled keys from being flagged as a
repeated stroke */
+                       __clear_bit(code, handle->dev->key);


Else drivers that use these bits for autorepeat (like the atkbd driver)
will end up with stuck keys.

> The right solution is to somehow hook at the atkbd driver , tell it that
> kbd tampered with its hardware, and make it release the keys.
>
>   

I don't think we need to do that because the input filter should be able
to actually "properly" filter state to all its clients.  As a last
resort of course the HW state could be tracked and updated, but ideally
I'd like to avoid it.

Jason.