[PATCH] ts-adc-debounce: Add delayed_pressure support

[Paul Sokolovsky]

Hello Gerhard,


        Ok, so we just handled samcop_adc brokenness issue, one off
the list. I propose now to handle "need for delayed pressure
processing" issue though, as we know that it's the real problem we
solve. Once we'll handle this, we'll come back to "allow interleaved
sampling" issue, to fully optimize and perfectalize stuff (in this
regard, your ts-adc-debounce.c.diff is much appreciated).

        The patch attached. Please review it, see if it does the right
thing, note that needed behavior is activated using
".delayed_pressure = 1", and test that it actually works.


Thanks!

--
Best regards,
 Paul                          mailto:pmiscml@...

_______________________________________________
Kernel-discuss mailing list
Kernel-discuss@...
https://handhelds.org/mailman/listinfo/kernel-discuss

[Gerhard Zintel]

Hello Paul,

On Tuesday 19 June 2007 01:01:16 Paul Sokolovsky wrote:
> Hello Gerhard,
>         The patch attached. Please review it, see if it does the right
> thing, note that needed behavior is activated using
> ".delayed_pressure = 1", and test that it actually works.

I have tested the patch with original ts-adc-debounce with xxxx|yyyy|zz format
for readings and it works very well! I can send debugging information later
on if anybody is interested. I propose one small change:

        if (params->delayed_pressure) {
                if (ts->sample_no > 0)
                        report_touchpanel(ts, ts->last_pos.xd, ts->last_pos.yd, 1);
                else
                        report_touchpanel(ts, ts_pos.xd, ts_pos.yd, 1);
                ts->last_pos = ts_pos;
        } else {
                report_touchpanel(ts, ts_pos.xd, ts_pos.yd, 1);
        }
        ts->sample_no++;

that would mean that in case of (sample_no == 0) the actual sample is used. I
expect this will improve responsivenes in case of very short pen downs. If
first sample gives good value and next sample already gives pen_up detection
wouldn't we miss the first measure otherwise? (I've not tested this proposal
yet.)

Now that pen_up detection is much improved we must decide how to go on. Sample
format xxxx|yyyy|zz is bad for Samcop device. It would be better to have xy
values sampled with same reading due to:
1. Samcop gives xyz values with one reading ==> more effective code
2. Two consecutive valid x readings are time shifted to valid y readings with
existing format

I can try to reduce num_xy_readings as much as possible and keep original
xxxx|yyyy|zz format so we don'd had to change ts-adc-debounce any further. Or
I can use my patch to let ts-adc-debounce request xy|xy|xy|zz format.

The patch is finished and works well but I have no good idea how to implement
it correctly, that is as an alternative to the original behaviour that should
be available for the other devices too. In the patch I already sent I
used "#define SAMCOP_ADC" for conditional compilation but that could not be
the final solution.

Gerhard
_______________________________________________
Kernel-discuss mailing list
Kernel-discuss@...
https://handhelds.org/mailman/listinfo/kernel-discuss

[Anton Vorontsov]

On Tue, Jun 19, 2007 at 07:16:50PM +0200, Gerhard Zintel wrote:
> I can try to reduce num_xy_readings as much as possible and keep original
> xxxx|yyyy|zz format so we don'd had to change ts-adc-debounce any further.

No no no. Please don't.

> Or
> I can use my patch to let ts-adc-debounce request xy|xy|xy|zz format.

Well, as I've said already, your patch is equivalent to one which is
already in CVS HEAD.

That patch _permits_ you doing optimized xy|xy|xyz|xyz readings.

> Gerhard

Thanks,

--
Anton Vorontsov
email: cbou@...
backup email: ya-cbou@...
irc://irc.freenode.org/bd2
_______________________________________________
Kernel-discuss mailing list
Kernel-discuss@...
https://handhelds.org/mailman/listinfo/kernel-discuss

[Anton Vorontsov]

On Wed, Jun 20, 2007 at 12:17:22AM +0400, Anton Vorontsov wrote:

> On Tue, Jun 19, 2007 at 07:16:50PM +0200, Gerhard Zintel wrote:
> > I can try to reduce num_xy_readings as much as possible and keep original
> > xxxx|yyyy|zz format so we don'd had to change ts-adc-debounce any further.
>
> No no no. Please don't.
>
> > Or
> > I can use my patch to let ts-adc-debounce request xy|xy|xy|zz format.
>
> Well, as I've said already, your patch is equivalent to one which is
> already in CVS HEAD.
>
> That patch _permits_ you doing optimized xy|xy|xyz|xyz readings.

I.e. please, publish your latest patch to ts-adc-debounce, I'll test
it on h5000. ;-)
 
Thanks,

--
Anton Vorontsov
email: cbou@...
backup email: ya-cbou@...
irc://irc.freenode.org/bd2
_______________________________________________
Kernel-discuss mailing list
Kernel-discuss@...
https://handhelds.org/mailman/listinfo/kernel-discuss

[Gerhard Zintel]

Hello Anton,

On Tuesday 19 June 2007 22:23:29 Anton Vorontsov wrote:

> On Wed, Jun 20, 2007 at 12:17:22AM +0400, Anton Vorontsov wrote:
> > On Tue, Jun 19, 2007 at 07:16:50PM +0200, Gerhard Zintel wrote:
> > > I can try to reduce num_xy_readings as much as possible and keep
> > > original xxxx|yyyy|zz format so we don'd had to change ts-adc-debounce
> > > any further.
> >
> > No no no. Please don't.
> >
> > > Or
> > > I can use my patch to let ts-adc-debounce request xy|xy|xy|zz format.
> >
> > Well, as I've said already, your patch is equivalent to one which is
> > already in CVS HEAD.

Here I spoke of the patch in ts-adc-debounce

> >
> > That patch _permits_ you doing optimized xy|xy|xyz|xyz readings.
>
> I.e. please, publish your latest patch to ts-adc-debounce, I'll test
> it on h5000. ;-)
Here you are, patches are attached.

A few remarks:

1.
meanwhile after giving results to Paul (timeout=7) I have commented line 139
in samcop_adc
  // printk(KERN_DEBUG "samcop_adc: timeout=%d\n", timeout);
as this line gives a whole bunch of output in dmesg file. Paul likes to know
if there is also output for h5000 device. Please tell him, after that I think
we should delete the printk command.

2.
for the time being there is a "#define SAMCOP_ADC" in file ts-adc-debounce for
conditional compilation of my patch. Without it ts-adc-debounce should behave
like before.

3.
Debug output now gives xyxyxy...z1z2z1z2 readings.

4.
Sampling could be a bit more compact if we change format to
xyxyxy...xyz1z2xyz1z2 and would have 10 readings in total inclusiv
Z_READINGS. I haven't done it yet.

5.
Patch works for me. As by now ten minutes of testing without one wrong pen_up
detection. But now I will compile again without defining DEBUG and
DEBUG_SAMPLES as it changes timing behaviour.

6.
Looking for a good way to code my patch into ts-adc-debounce without the
clumsy "#ifdefine SAMCOP_ADC" method. I could make use of delayed_pressure as
by now it is only set if Samcop device is used but it seems not correct
either.

Thank you
Gerhard

[hamcop_base.c.diff]

---

Index: drivers/mfd/hamcop_base.c
===================================================================
RCS file: /cvs/linux/kernel26/drivers/mfd/hamcop_base.c,v
retrieving revision 1.72
diff -u -d -b -B -r1.72 hamcop_base.c
--- drivers/mfd/hamcop_base.c 8 Jun 2007 19:31:22 -0000 1.72
+++ drivers/mfd/hamcop_base.c 19 Jun 2007 21:09:53 -0000
@@ -1113,6 +1113,7 @@
  .z1_pin = "samcop adc:z1",
  .z2_pin = "samcop adc:z2",
  .max_jitter = 10,
+ .delayed_pressure = 1,
 };
 
 struct ds1wm_platform_data hamcop_ds1wm_plat_data = {

---

[samcop_base.c.diff]

---

Index: drivers/mfd/samcop_base.c
===================================================================
RCS file: /cvs/linux/kernel26/drivers/mfd/samcop_base.c,v
retrieving revision 1.104
diff -u -d -b -B -r1.104 samcop_base.c
--- drivers/mfd/samcop_base.c 26 May 2007 15:46:56 -0000 1.104
+++ drivers/mfd/samcop_base.c 19 Jun 2007 21:09:44 -0000
@@ -1094,6 +1094,7 @@
  .z1_pin = "samcop adc:z1",
  .z2_pin = "samcop adc:z2",
  .max_jitter = 10,
+ .delayed_pressure = 1,
 };
 
 struct samcop_block

---

[ts-adc-debounce.c.diff]

---

Index: drivers/input/touchscreen/ts-adc-debounce.c
===================================================================
RCS file: /cvs/linux/kernel26/drivers/input/touchscreen/ts-adc-debounce.c,v
retrieving revision 1.53
diff -u -d -b -B -r1.53 ts-adc-debounce.c
--- drivers/input/touchscreen/ts-adc-debounce.c 18 Jun 2007 22:36:33 -0000 1.53
+++ drivers/input/touchscreen/ts-adc-debounce.c 19 Jun 2007 21:07:18 -0000
@@ -13,8 +13,9 @@
  *
  */
 
-//#define DEBUG
-//#define DEBUG_SAMPLES
+#define DEBUG
+#define DEBUG_SAMPLES
+#define SAMCOP_ADC
 
 #define DRIVER_NAME "ts-adc-debounce"
 
@@ -51,11 +52,20 @@
  struct platform_device *pdev;
 };
 
+struct ts_pos {
+ unsigned long xd;
+ unsigned long yd;
+ unsigned int pressure;
+ int err;
+};
+
 struct ts_adc {
  unsigned int pen_irq;
  struct input_dev *input;
  // Is touchscreen active.
  int state;
+ int sample_no;
+ struct ts_pos last_pos;
 
  struct adc_request req;
  struct adc_sense *pins;
@@ -66,13 +76,6 @@
 
 #define delta_within_bounds(x,y,d) ( ((int)((x) - (y)) < (int)(d)) && ((int)((y) - (x)) < (int)(d)) )
 
-struct ts_pos {
-    unsigned long xd;
-    unsigned long yd;
-    unsigned int pressure;
-    int err;
-};
-
 static void report_touchpanel(struct ts_adc *ts, int x, int y, int pressure)
 {
  input_report_key(ts->input, BTN_TOUCH, pressure != 0);
@@ -84,9 +87,71 @@
  input_sync(ts->input);
 }
 
+#ifdef SAMCOP_ADC
 void debounce_samples(struct tsadc_platform_data *params, struct adc_sense *pins, struct ts_pos *tp)
 {
- unsigned int t, x, y, z1, z2, pressure = 0;
+ unsigned int t1, t2, x, y, z1, z2, pressure = 0;
+ int i, ptr;
+ int d = params->max_jitter;
+
+#ifdef DEBUG_SAMPLES
+ pr_debug(DRIVER_NAME ": ");
+ ptr = 0;
+ for (i = 0; i < 2*params->num_xy_samples; i++) {
+ printk("%04d ", pins[ptr++].value);
+ }
+ pr_debug("| ");
+ for (i = 0; i < 2*Z_READINGS; i++) {
+ printk("%04d ", pins[ptr++].value);
+ }
+ pr_debug("\n");
+#endif
+
+ /* X/Y-axis */
+ ptr = 0;
+ x = pins[ptr++].value;
+ y = pins[ptr++].value;
+ /* Run thru samples and see if they converge */
+ for (i = params->num_xy_samples - 1; i > 0; i--) {
+ t1 = x;
+ t2 = y;
+ x = pins[ptr++].value;
+ y = pins[ptr++].value;
+ if ((delta_within_bounds(t1, x, d)) &&
+   (delta_within_bounds(t2, y, d)) )
+ break;
+ }
+ /* If we exhausted loop, it means the samples unconverged */
+ if (i == 0)
+ tp->err = 1;
+
+ /* Pressure */
+ ptr = params->num_xy_samples * 2;
+ /* Discard first sample as unstable */
+ ptr++;
+ ptr++;
+ z1 = pins[ptr++].value;
+ z2 = pins[ptr++].value;
+
+ // RTOUCH = (RXPlate) x (XPOSITION /4096) x [(Z2/Z1) - 1]
+ // pressure is proportional to 1 / RTOUCH
+ pressure = abs(z2 - z1) * (x ? x : 1); // X just shouldn't be 0
+ if (pressure != 0)
+ pressure = params->pressure_factor * z1 / pressure;
+ else
+ //pressure = BIG_VALUE; // this regresses h5000. workaround is below. needs more investigation
+ pressure = z1;
+
+ pr_debug(DRIVER_NAME ": x=%d y=%d z1=%d z2=%d P=%d\n", x, y, z1, z2, pressure);
+
+ tp->xd = x;
+ tp->yd = y;
+ tp->pressure = pressure;
+}
+#else
+void debounce_samples(struct tsadc_platform_data *params, struct adc_sense *pins, struct ts_pos *tp)
+{
+ unsigned int t1, t2, x, y, z1, z2, pressure = 0;
  int i, ptr;
  int d = params->max_jitter;
 
@@ -161,6 +226,7 @@
  tp->yd = y;
  tp->pressure = pressure;
 }
+#endif
 
 static irqreturn_t ts_adc_debounce_isr(int irq, void* data)
 {
@@ -182,6 +248,7 @@
  disable_irq_nosync(ts->pen_irq);
 
  ts->state = STATE_SAMPLING;
+ ts->sample_no = 0;
  pr_debug(DRIVER_NAME ": Started to sample\n");
  cancel_delayed_work(&ts->work.work);
  queue_delayed_work(ts->wq, &ts->work.work, 0);
@@ -227,8 +294,18 @@
  //printk("%04d %04d\n", (int)ts_pos.xd, (int)ts_pos.yd);
  if (ts_pos.err)
  printk(DRIVER_NAME ": Sample with too much jitter - ignored\n");
+ else {
+ if (params->delayed_pressure) {
+ if (ts->sample_no > 0)
+ report_touchpanel(ts, ts->last_pos.xd, ts->last_pos.yd, 1);
  else
  report_touchpanel(ts, ts_pos.xd, ts_pos.yd, 1);
+ ts->last_pos = ts_pos;
+ } else {
+ report_touchpanel(ts, ts_pos.xd, ts_pos.yd, 1);
+ }
+ ts->sample_no++;
+ }
 
  queue_delayed_work(ts->wq, &ts->work.work, (SAMPLE_TIMEOUT * HZ) / 1000);
  }
@@ -236,12 +313,24 @@
  return;
 };
 
+#ifdef SAMCOP_ADC
+inline static void fill_pins(struct adc_sense *pins, int from, int num,
+ char *with1, char *with2)
+{
+ int i, ptr=from;
+ for (i = from + num - 1; i >= from; i--) {
+ pins[ptr++].name = with1;
+ pins[ptr++].name = with2;
+ }
+}
+#else
 inline static void fill_pins(struct adc_sense *pins, int from, int num, char *with)
 {
  int i;
  for (i = from + num - 1; i >= from; i--)
  pins[i].name = with;
 }
+#endif
 
 static int ts_adc_debounce_probe(struct platform_device *pdev)
 {
@@ -262,12 +351,18 @@
  ts->req.senses = ts->pins;
  ts->req.num_senses = (pdata->num_xy_samples*2 + Z_READINGS*2);
  i = 0;
+#ifdef SAMCOP_ADC
+ fill_pins(ts->pins, i, pdata->num_xy_samples, pdata->x_pin, pdata->y_pin);
+ fill_pins(ts->pins, i += 2*pdata->num_xy_samples,
+  Z_READINGS, pdata->z1_pin, pdata->z2_pin);
+#else
  fill_pins(ts->pins, i, pdata->num_xy_samples, pdata->x_pin);
  fill_pins(ts->pins, i += pdata->num_xy_samples,
  pdata->num_xy_samples, pdata->y_pin);
  fill_pins(ts->pins, i += pdata->num_xy_samples,  
  Z_READINGS, pdata->z1_pin);
  fill_pins(ts->pins, i +=  Z_READINGS,  Z_READINGS, pdata->z2_pin);
+#endif
  adc_request_register(&ts->req);
 
  ts->work.pdev = pdev;

---

_______________________________________________
Kernel-discuss mailing list
Kernel-discuss@...
https://handhelds.org/mailman/listinfo/kernel-discuss

[Gerhard Zintel]

Anton,

On Tuesday 19 June 2007 22:23:29 Anton Vorontsov wrote:
> I.e. please, publish your latest patch to ts-adc-debounce, I'll test
> it on h5000. ;-)

One remark I forgott to mention in my last post:
How will you test the patch? Are you using Opie or GPE environment? In Opie
(that's what I'm using) pen_up problem is very anoying. You will see the
difference at once. For GPE (==> X11) I have heard that this pen_up problem
does not appear as X11 uses better strategie - but I'm not sure.

Gerhard

_______________________________________________
Kernel-discuss mailing list
Kernel-discuss@...
https://handhelds.org/mailman/listinfo/kernel-discuss

[Paul Sokolovsky]

Hello Gerhard,

Tuesday, June 19, 2007, 8:16:50 PM, you wrote:

> Hello Paul,

> On Tuesday 19 June 2007 01:01:16 Paul Sokolovsky wrote:
>> Hello Gerhard,
>>         The patch attached. Please review it, see if it does the right
>> thing, note that needed behavior is activated using
>> ".delayed_pressure = 1", and test that it actually works.

> I have tested the patch with original ts-adc-debounce with xxxx|yyyy|zz format
> for readings and it works very well! I can send debugging information later
> on if anybody is interested. I propose one small change:

>         if (params->delayed_pressure) {
>                 if (ts->sample_no > 0)
>                         report_touchpanel(ts, ts->last_pos.xd, ts->last_pos.yd, 1);
>                 else
>                         report_touchpanel(ts, ts_pos.xd, ts_pos.yd, 1);
>                 ts->last_pos = ts_pos;
>         } else {
>                 report_touchpanel(ts, ts_pos.xd, ts_pos.yd, 1);
>         }
>         ts->sample_no++;

> that would mean that in case of (sample_no == 0) the actual sample is used. I
> expect this will improve responsivenes in case of very short pen downs. If
> first sample gives good value and next sample already gives pen_up detection
> wouldn't we miss the first measure otherwise? (I've not tested this proposal
> yet.)

        Make sense, committed with this change.


        Will review what we have with the last problem later...

[]

--
Best regards,
 Paul                            mailto:pmiscml@...

_______________________________________________
Kernel-discuss mailing list
Kernel-discuss@...
https://handhelds.org/mailman/listinfo/kernel-discuss

[Paul Sokolovsky]

Hello Gerhard,


        Ok, now that 2 previous issues are done, off for last one.

Tuesday, June 19, 2007, 8:16:50 PM, you wrote:

[]

> Now that pen_up detection is much improved we must decide how to go on. Sample
> format xxxx|yyyy|zz is bad for Samcop device. It would be better to have xy
> values sampled with same reading due to:
> 1. Samcop gives xyz values with one reading ==> more effective code
> 2. Two consecutive valid x readings are time shifted to valid y readings with
> existing format

> I can try to reduce num_xy_readings as much as possible and keep original
> xxxx|yyyy|zz format so we don'd had to change ts-adc-debounce any further. Or
> I can use my patch to let ts-adc-debounce request xy|xy|xy|zz format.

> The patch is finished and works well but I have no good idea how to implement
> it correctly, that is as an alternative to the original behaviour that should
> be available for the other devices too. In the patch I already sent I
> used "#define SAMCOP_ADC" for conditional compilation but that could not be
> the final solution.


1. Let me start with the last querying if we really need any
debouncing with samcop/hamcop. Logs posted by Gerhardshow that in most
cases readings are very stable, and when they are not, that's pen up cases,
which we should have handled by now. Anton, your call.

2. Otherwise, if we really need debouncing support for xy|xy samplings,
patch prepared by Gerhard shows that there's indeed much of duplicated code
for very slightly different algorithms. I'd like to re-raise question
if we'd better have one generic algorithm able to handle any sample
orders. One issue I see is that there's a way to distinguish samples of
different signals from each other, but no easy way to tell which
sample is which. I don't consider string manipulation the easy way in
this regard. Anton, what would you suggest? I may imagine preparing (sorry,
it's still called "registering") fake request for the sole purpose of
inferring pin -> ping_id mapping. Measures pretty hacky on my scale
though, who introduced those strings in kernel space? ;-)

        Comments welcome.


> Gerhard



--
Best regards,
 Paul                            mailto:pmiscml@...

_______________________________________________
Kernel-discuss mailing list
Kernel-discuss@...
https://handhelds.org/mailman/listinfo/kernel-discuss

[Gerhard Zintel]

Hello Paul,

On Wednesday 20 June 2007 00:22:13 Paul Sokolovsky wrote:
>
> 1. Let me start with the last querying if we really need any
> debouncing with samcop/hamcop. Logs posted by Gerhardshow that in most
> cases readings are very stable, and when they are not, that's pen up cases,
> which we should have handled by now. Anton, your call.
>
I will try if h2200 is working well with only one xyz reading (thus without
any debouncing). This would give minimal changes in ts-adc-debounce and we
can use num_xy_readings as an indicator.

> 2. Otherwise, if we really need debouncing support for xy|xy samplings,
> patch prepared by Gerhard shows that there's indeed much of duplicated code
> for very slightly different algorithms. I'd like to re-raise question
> if we'd better have one generic algorithm able to handle any sample
> orders. One issue I see is that there's a way to distinguish samples of
> different signals from each other, but no easy way to tell which
> sample is which. I don't consider string manipulation the easy way in
> this regard. Anton, what would you suggest? I may imagine preparing (sorry,
> it's still called "registering") fake request for the sole purpose of
> inferring pin -> ping_id mapping. Measures pretty hacky on my scale
> though, who introduced those strings in kernel space? ;-)

Here I'm lost. What are samples of different signals?

Gerhard
_______________________________________________
Kernel-discuss mailing list
Kernel-discuss@...
https://handhelds.org/mailman/listinfo/kernel-discuss

[Gerhard Zintel]

Hello Paul,

to finish the "debounce" problem for h2200 I'd like to open this discussion
again.

Independent of the rewrite of ts-adc-debounce we need to
set ".delayed_pressure = 1" for h2200 devices and the like in hamcop_base.c.
Thats the reason for the first small patch I've attached
(hamcop_base.c.diff). I assume it should be changed in samcop_base.c too but
I can not test it.

Another small patch is for samcop_adc.c. With each reading it writes the
measured timeout value into dmesg file. We should avoid this. I've already
told you that for h2200 this value is 7 (and sometimes 8). The patch
samcop_adc.diff changes the appropriate printk statement to pr_debug.


On Wednesday 20 June 2007 00:22:13 Paul Sokolovsky wrote:
>
> 1. Let me start with the last querying if we really need any
> debouncing with samcop/hamcop. Logs posted by Gerhardshow that in most
> cases readings are very stable, and when they are not, that's pen up cases,
> which we should have handled by now. Anton, your call.

Next thing I've done is to set "num_xy_samples = 1" in hamcop_base.c and
"#define Z_READINGS 1" in ts-adc-debounce.c. With a few more modifications in
ts-adc-debounce h2200 runs with only 1 reading per sample. I have tested it a
few hours without any glitch. The display feels very responsive. Thus my
proposal is, to go this way.

I've attached patches hamcop_base.c.diff2 and ts-adc-debounce.c.diff for those
who want to try it.

Sure, going this way would mean we wouldn't do any debouncing at all - but if
the result is OK we should go this way.

If you are fine with this I would add num_z_samples variable in
tsadc_platform_data make the final modifications in ts-adc-debounce and send
you the final patch.

Regards
Gerhard



[hamcop_base.c.diff]

---

Index: drivers/mfd/hamcop_base.c
===================================================================
RCS file: /cvs/linux/kernel26/drivers/mfd/hamcop_base.c,v
retrieving revision 1.72
diff -u -d -b -B -r1.72 hamcop_base.c
--- drivers/mfd/hamcop_base.c 8 Jun 2007 19:31:22 -0000 1.72
+++ drivers/mfd/hamcop_base.c 26 Jun 2007 18:45:33 -0000
@@ -1112,6 +1112,7 @@
  .y_pin = "samcop adc:y",
  .z1_pin = "samcop adc:z1",
  .z2_pin = "samcop adc:z2",
+ .delayed_pressure = 1,
  .max_jitter = 10,
 };
 

---

[hamcop_base.c.diff2]

---

Index: drivers/mfd/hamcop_base.c
===================================================================
RCS file: /cvs/linux/kernel26/drivers/mfd/hamcop_base.c,v
retrieving revision 1.72
diff -u -d -b -B -r1.72 hamcop_base.c
--- drivers/mfd/hamcop_base.c 8 Jun 2007 19:31:22 -0000 1.72
+++ drivers/mfd/hamcop_base.c 26 Jun 2007 18:57:17 -0000
@@ -1112,6 +1112,8 @@
  .y_pin = "samcop adc:y",
  .z1_pin = "samcop adc:z1",
  .z2_pin = "samcop adc:z2",
+ .num_xy_samples = 1,
+ .delayed_pressure = 1,
  .max_jitter = 10,
 };
 

---

[ts-adc-debounce.c.diff]

---

Index: drivers/input/touchscreen/ts-adc-debounce.c
===================================================================
RCS file: /cvs/linux/kernel26/drivers/input/touchscreen/ts-adc-debounce.c,v
retrieving revision 1.54
diff -u -d -b -B -r1.54 ts-adc-debounce.c
--- drivers/input/touchscreen/ts-adc-debounce.c 19 Jun 2007 21:55:23 -0000 1.54
+++ drivers/input/touchscreen/ts-adc-debounce.c 26 Jun 2007 18:59:19 -0000
@@ -35,7 +35,7 @@
 #include <asm/irq.h>
 #include <asm/mach/irq.h>
 
-#define Z_READINGS 2
+#define Z_READINGS 1
 
 #define BIG_VALUE 999999
 
@@ -116,6 +116,7 @@
  /* X-axis */
  ptr = 0;
  x = pins[ptr++].value;
+ if (params->num_xy_samples > 1) {
  /* Run thru samples and see if they converge */
  for (i = params->num_xy_samples - 1; i > 0; i--) {
  t = x;
@@ -123,13 +124,16 @@
  if (delta_within_bounds(t, x, d))
  break;
  }
+
  /* If we exhausted loop, it means the samples unconverged */
  if (i == 0)
  tp->err = 1;
+ }
 
  /* Y-axis */
  ptr = params->num_xy_samples;
  y = pins[ptr++].value;
+ if (params->num_xy_samples > 1) {
  for (i = params->num_xy_samples - 1; i > 0; i--) {
  t = y;
  y = pins[ptr++].value;
@@ -138,14 +142,15 @@
  }
  if (i == 0)
  tp->err = 1;
+ }
 
  /* Pressure */
  ptr = params->num_xy_samples * 2;
  /* Discard first sample as unstable */
- ptr++;
+ if (Z_READINGS > 1) ptr++;
  z1 = pins[ptr++].value;
 
- ptr++;
+ if (Z_READINGS > 1) ptr++;
  z2 = pins[ptr++].value;
 
  // RTOUCH = (RXPlate) x (XPOSITION /4096) x [(Z2/Z1) - 1]

---

[samcop_adc.c.diff]

---

Index: drivers/misc/adc/samcop_adc.c
===================================================================
RCS file: /cvs/linux/kernel26/drivers/misc/adc/samcop_adc.c,v
retrieving revision 1.37
diff -u -d -b -B -r1.37 samcop_adc.c
--- drivers/misc/adc/samcop_adc.c 18 Jun 2007 22:11:55 -0000 1.37
+++ drivers/misc/adc/samcop_adc.c 26 Jun 2007 19:07:29 -0000
@@ -136,7 +136,7 @@
  }
  udelay(50);
  }
- printk(KERN_DEBUG "samcop_adc: timeout=%d\n", timeout);
+ pr_debug(KERN_DEBUG "samcop_adc: timeout=%d\n", timeout);
 
  return 0;
 }

---

_______________________________________________
Kernel-discuss mailing list
Kernel-discuss@...
https://handhelds.org/mailman/listinfo/kernel-discuss

[Paul Sokolovsky]

Hello Gerhard,

Tuesday, June 26, 2007, 10:23:37 PM, you wrote:

> Hello Paul,

> to finish the "debounce" problem for h2200 I'd like to open this discussion
> again.

> Independent of the rewrite of ts-adc-debounce we need to
> set ".delayed_pressure = 1" for h2200 devices and the like in hamcop_base.c.
> Thats the reason for the first small patch I've attached
> (hamcop_base.c.diff).

        Committed, thanks.

> I assume it should be changed in samcop_base.c too but
> I can not test it.

        Yes, let's leave that to Anton/Milan, but I assume TS on h5000
works well enough for now, so there's no haste with this.

> Another small patch is for samcop_adc.c. With each reading it writes the
> measured timeout value into dmesg file. We should avoid this. I've already
> told you that for h2200 this value is 7 (and sometimes 8). The patch
> samcop_adc.diff changes the appropriate printk statement to pr_debug.

        Ok, I just commented that, it served it's purpose. Anton
confirmed that it's 0 on samcop.


> On Wednesday 20 June 2007 00:22:13 Paul Sokolovsky wrote:
>>
>> 1. Let me start with the last querying if we really need any
>> debouncing with samcop/hamcop. Logs posted by Gerhardshow that in most
>> cases readings are very stable, and when they are not, that's pen up cases,
>> which we should have handled by now. Anton, your call.

> Next thing I've done is to set "num_xy_samples = 1" in hamcop_base.c and
> "#define Z_READINGS 1" in ts-adc-debounce.c. With a few more modifications in
> ts-adc-debounce h2200 runs with only 1 reading per sample. I have tested it a
> few hours without any glitch. The display feels very responsive. Thus my
> proposal is, to go this way.

> I've attached patches hamcop_base.c.diff2 and ts-adc-debounce.c.diff for those
> who want to try it.

> Sure, going this way would mean we wouldn't do any debouncing at all - but if
> the result is OK we should go this way.

        Yes, I agree. I actually wait for some definitive answer from
Anton if that will work for h5000 too. If so, we won't need to
implement interleaved pattern at all for now.

> If you are fine with this I would add num_z_samples variable in
> tsadc_platform_data make the final modifications in ts-adc-debounce and send
> you the final patch.

        Yes, good plan, please implement that. Handling of
num_xy_samples = 1 is committed in the meantime.

> Regards
> Gerhard





--
Best regards,
 Paul                            mailto:pmiscml@...

_______________________________________________
Kernel-discuss mailing list
Kernel-discuss@...
https://handhelds.org/mailman/listinfo/kernel-discuss

[Anton Vorontsov]

On Tue, Jun 26, 2007 at 09:23:37PM +0200, Gerhard Zintel wrote:

> Hello Paul,
>
> to finish the "debounce" problem for h2200 I'd like to open this discussion
> again.
>
> Independent of the rewrite of ts-adc-debounce we need to
> set ".delayed_pressure = 1" for h2200 devices and the like in hamcop_base.c.
> Thats the reason for the first small patch I've attached
> (hamcop_base.c.diff). I assume it should be changed in samcop_base.c too but
> I can not test it.
>
> Another small patch is for samcop_adc.c. With each reading it writes the
> measured timeout value into dmesg file. We should avoid this. I've already
> told you that for h2200 this value is 7 (and sometimes 8). The patch
> samcop_adc.diff changes the appropriate printk statement to pr_debug.
>
>
> On Wednesday 20 June 2007 00:22:13 Paul Sokolovsky wrote:
> >
> > 1. Let me start with the last querying if we really need any
> > debouncing with samcop/hamcop. Logs posted by Gerhardshow that in most
> > cases readings are very stable, and when they are not, that's pen up cases,
> > which we should have handled by now. Anton, your call.
>
> Next thing I've done is to set "num_xy_samples = 1" in hamcop_base.c and
> "#define Z_READINGS 1" in ts-adc-debounce.c. With a few more modifications in
> ts-adc-debounce h2200 runs with only 1 reading per sample. I have tested it a
> few hours without any glitch.

Great job, Gerhard!

Few cosmetic comments..

> Index: drivers/input/touchscreen/ts-adc-debounce.c
> ===================================================================
> RCS file: /cvs/linux/kernel26/drivers/input/touchscreen/ts-adc-debounce.c,v
> retrieving revision 1.54
> diff -u -d -b -B -r1.54 ts-adc-debounce.c
> --- drivers/input/touchscreen/ts-adc-debounce.c 19 Jun 2007 21:55:23 -0000 1.54
> +++ drivers/input/touchscreen/ts-adc-debounce.c 26 Jun 2007 18:59:19 -0000
> @@ -35,7 +35,7 @@
>  #include <asm/irq.h>
>  #include <asm/mach/irq.h>
>  
> -#define Z_READINGS 2
> +#define Z_READINGS 1

Can pass this via platform data, as done for xy readings?

>  
>  #define BIG_VALUE 999999
>  
> @@ -116,6 +116,7 @@
>   /* X-axis */
>   ptr = 0;
>   x = pins[ptr++].value;
> + if (params->num_xy_samples > 1) {
>   /* Run thru samples and see if they converge */

1. Please, fix indentation. I suppose you haven't done this by
intention, to show exactly what changed. But for final version,
this should be fixed.
2. I'd like to see comment above that if-statement. Something like
"/* Do real debouncing only if multiple xy samples requested. */".

>   for (i = params->num_xy_samples - 1; i > 0; i--) {
>   t = x;
> @@ -123,13 +124,16 @@
>   if (delta_within_bounds(t, x, d))
>   break;
>   }
> +
>   /* If we exhausted loop, it means the samples unconverged */
>   if (i == 0)
>   tp->err = 1;
> + }
>  
>   /* Y-axis */
>   ptr = params->num_xy_samples;
>   y = pins[ptr++].value;
> + if (params->num_xy_samples > 1) {

1.
2.

>   for (i = params->num_xy_samples - 1; i > 0; i--) {
>   t = y;
>   y = pins[ptr++].value;
> @@ -138,14 +142,15 @@
>   }
>   if (i == 0)
>   tp->err = 1;
> + }
>  
>   /* Pressure */
>   ptr = params->num_xy_samples * 2;
>   /* Discard first sample as unstable */
> - ptr++;
> + if (Z_READINGS > 1) ptr++;

if (foo)
        bar();

>   z1 = pins[ptr++].value;
>  
> - ptr++;
> + if (Z_READINGS > 1) ptr++;
>   z2 = pins[ptr++].value;
>  
>   // RTOUCH = (RXPlate) x (XPOSITION /4096) x [(Z2/Z1) - 1]

> Index: drivers/misc/adc/samcop_adc.c
> ===================================================================
> RCS file: /cvs/linux/kernel26/drivers/misc/adc/samcop_adc.c,v
> retrieving revision 1.37
> diff -u -d -b -B -r1.37 samcop_adc.c
> --- drivers/misc/adc/samcop_adc.c 18 Jun 2007 22:11:55 -0000 1.37
> +++ drivers/misc/adc/samcop_adc.c 26 Jun 2007 19:07:29 -0000
> @@ -136,7 +136,7 @@
>   }
>   udelay(50);
>   }
> - printk(KERN_DEBUG "samcop_adc: timeout=%d\n", timeout);
> + pr_debug(KERN_DEBUG "samcop_adc: timeout=%d\n", timeout);

No need for KERN_DEBUG, pr_debug is enough.


Otherwise looks perfect.

Thanks,

--
Anton Vorontsov
email: cbou@...
backup email: ya-cbou@...
irc://irc.freenode.org/bd2
_______________________________________________
Kernel-discuss mailing list
Kernel-discuss@...
https://handhelds.org/mailman/listinfo/kernel-discuss

[Gerhard Zintel]

Here you are,

patch for final "debouncing" of h2200 devices attached.

Anton, I tried to follow your cosmetic comments in the other mail. Hope I've
got them.

Patch is tested on my h2200 and works well.

Michal, please give it a try also.

Thank you for all your help
Gerhard


On Tuesday 26 June 2007 22:22:26 Paul Sokolovsky wrote:

> Hello Gerhard,
>
> Tuesday, June 26, 2007, 10:23:37 PM, you wrote:
> > Hello Paul,
> >
> > to finish the "debounce" problem for h2200 I'd like to open this
> > discussion again.
> >
> > Independent of the rewrite of ts-adc-debounce we need to
> > set ".delayed_pressure = 1" for h2200 devices and the like in
> > hamcop_base.c. Thats the reason for the first small patch I've attached
> > (hamcop_base.c.diff).
>
>         Committed, thanks.
>
> > I assume it should be changed in samcop_base.c too but
> > I can not test it.
>
>         Yes, let's leave that to Anton/Milan, but I assume TS on h5000
> works well enough for now, so there's no haste with this.
>
> > Another small patch is for samcop_adc.c. With each reading it writes the
> > measured timeout value into dmesg file. We should avoid this. I've
> > already told you that for h2200 this value is 7 (and sometimes 8). The
> > patch samcop_adc.diff changes the appropriate printk statement to
> > pr_debug.
>
>         Ok, I just commented that, it served it's purpose. Anton
> confirmed that it's 0 on samcop.
>
> > On Wednesday 20 June 2007 00:22:13 Paul Sokolovsky wrote:
> >> 1. Let me start with the last querying if we really need any
> >> debouncing with samcop/hamcop. Logs posted by Gerhardshow that in most
> >> cases readings are very stable, and when they are not, that's pen up
> >> cases, which we should have handled by now. Anton, your call.
> >
> > Next thing I've done is to set "num_xy_samples = 1" in hamcop_base.c and
> > "#define Z_READINGS 1" in ts-adc-debounce.c. With a few more
> > modifications in ts-adc-debounce h2200 runs with only 1 reading per
> > sample. I have tested it a few hours without any glitch. The display
> > feels very responsive. Thus my proposal is, to go this way.
> >
> > I've attached patches hamcop_base.c.diff2 and ts-adc-debounce.c.diff for
> > those who want to try it.
> >
> > Sure, going this way would mean we wouldn't do any debouncing at all -
> > but if the result is OK we should go this way.
>
>         Yes, I agree. I actually wait for some definitive answer from
> Anton if that will work for h5000 too. If so, we won't need to
> implement interleaved pattern at all for now.
>
> > If you are fine with this I would add num_z_samples variable in
> > tsadc_platform_data make the final modifications in ts-adc-debounce and
> > send you the final patch.
>
>         Yes, good plan, please implement that. Handling of
> num_xy_samples = 1 is committed in the meantime.
>
> > Regards
> > Gerhard

[h2200-debounce.diff]

---

Index: drivers/input/touchscreen/ts-adc-debounce.c
===================================================================
RCS file: /cvs/linux/kernel26/drivers/input/touchscreen/ts-adc-debounce.c,v
retrieving revision 1.56
diff -u -d -b -B -r1.56 ts-adc-debounce.c
--- drivers/input/touchscreen/ts-adc-debounce.c 26 Jun 2007 20:21:01 -0000 1.56
+++ drivers/input/touchscreen/ts-adc-debounce.c 26 Jun 2007 22:02:22 -0000
@@ -35,8 +35,6 @@
 #include <asm/irq.h>
 #include <asm/mach/irq.h>
 
-#define Z_READINGS 2
-
 #define BIG_VALUE 999999
 
 #define SAMPLE_TIMEOUT 20 /* sample every 20ms */
@@ -103,11 +101,11 @@
  printk("%04d ", pins[ptr++].value);
  }
  printk("| ");
- for (i = 0; i < Z_READINGS; i++) {
+ for (i = 0; i < params->num_z_samples; i++) {
  printk("%04d ", pins[ptr++].value);
  }
  printk("| ");
- for (i = 0; i < Z_READINGS; i++) {
+ for (i = 0; i < params->num_z_samples; i++) {
  printk("%04d ", pins[ptr++].value);
  }
  printk("\n");
@@ -116,6 +114,7 @@
  /* X-axis */
  ptr = 0;
  x = pins[ptr++].value;
+ /* Do real debouncing only if multiple xy samples requested. */
  if (params->num_xy_samples > 1) {
  /* Run thru samples and see if they converge */
  for (i = params->num_xy_samples - 1; i > 0; i--) {
@@ -132,6 +131,7 @@
  /* Y-axis */
  ptr = params->num_xy_samples;
  y = pins[ptr++].value;
+ /* Do real debouncing only if multiple xy samples requested. */
  if (params->num_xy_samples > 1) {
  for (i = params->num_xy_samples - 1; i > 0; i--) {
  t = y;
@@ -145,10 +145,12 @@
 
  /* Pressure */
  ptr = params->num_xy_samples * 2;
- /* Discard first sample as unstable */
+ /* Discard first sample as unstable if more than one z reading requested */
+ if (params->num_z_samples > 1)
  ptr++;
  z1 = pins[ptr++].value;
 
+ if (params->num_z_samples > 1)
  ptr++;
  z2 = pins[ptr++].value;
 
@@ -274,17 +276,18 @@
  platform_set_drvdata(pdev, ts);
 
  pdata->num_xy_samples = pdata->num_xy_samples ?: 10;
+ pdata->num_z_samples  = pdata->num_xy_samples ?: 2;
 
- ts->pins = kmalloc(sizeof(*ts->pins) * (pdata->num_xy_samples*2 + Z_READINGS*2), GFP_KERNEL);
+ ts->pins = kmalloc(sizeof(*ts->pins) * (pdata->num_xy_samples*2 + pdata->num_z_samples*2), GFP_KERNEL);
  ts->req.senses = ts->pins;
- ts->req.num_senses = (pdata->num_xy_samples*2 + Z_READINGS*2);
+ ts->req.num_senses = (pdata->num_xy_samples*2 + pdata->num_z_samples*2);
  i = 0;
  fill_pins(ts->pins, i, pdata->num_xy_samples, pdata->x_pin);
  fill_pins(ts->pins, i += pdata->num_xy_samples,
  pdata->num_xy_samples, pdata->y_pin);
  fill_pins(ts->pins, i += pdata->num_xy_samples,  
- Z_READINGS, pdata->z1_pin);
- fill_pins(ts->pins, i +=  Z_READINGS,  Z_READINGS, pdata->z2_pin);
+ pdata->num_z_samples, pdata->z1_pin);
+ fill_pins(ts->pins, i +=  pdata->num_z_samples,  pdata->num_z_samples, pdata->z2_pin);
  adc_request_register(&ts->req);
 
  ts->work.pdev = pdev;
Index: include/linux/touchscreen-adc.h
===================================================================
RCS file: /cvs/linux/kernel26/include/linux/touchscreen-adc.h,v
retrieving revision 1.10
diff -u -d -b -B -r1.10 touchscreen-adc.h
--- include/linux/touchscreen-adc.h 19 Jun 2007 21:55:23 -0000 1.10
+++ include/linux/touchscreen-adc.h 26 Jun 2007 22:02:23 -0000
@@ -30,6 +30,8 @@
  // Debouncing parameters
  /* Number of X/Y samplings done for one measurement */
  int num_xy_samples;
+ /* Number of Z1/Z2 samplings done for one measurement */
+ int num_z_samples;
  /* Samples during measurement must converge to be within bounds
    of this value, or the whole measurement will be discarded. */
  int max_jitter;
Index: drivers/mfd/hamcop_base.c
===================================================================
RCS file: /cvs/linux/kernel26/drivers/mfd/hamcop_base.c,v
retrieving revision 1.74
diff -u -d -b -B -r1.74 hamcop_base.c
--- drivers/mfd/hamcop_base.c 26 Jun 2007 20:07:46 -0000 1.74
+++ drivers/mfd/hamcop_base.c 26 Jun 2007 22:02:24 -0000
@@ -1111,6 +1111,9 @@
  .y_pin = "samcop adc:y",
  .z1_pin = "samcop adc:z1",
  .z2_pin = "samcop adc:z2",
+ .num_xy_samples = 1,
+ .num_z_samples = 1,
+ .delayed_pressure = 1,
  .max_jitter = 10,
  .delayed_pressure = 1,
 };

---

_______________________________________________
Kernel-discuss mailing list
Kernel-discuss@...
https://handhelds.org/mailman/listinfo/kernel-discuss