| Message ID | 20210827211258.318618-1-marex@denx.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/3] Input: ili210x - use resolution from ili251x firmware | expand |
Hi Marek, On Fri, Aug 27, 2021 at 11:12:56PM +0200, Marek Vasut wrote: > The ili251x firmware protocol permits readout of panel resolution, > implement this, but make it possible to override this value using > DT bindings. This way, older DTs which contain touchscreen-size-x > and touchscreen-size-y properties will behave just like before and > new DTs may avoid specifying these for ILI251x. > > Note that the command format is different on other controllers, so > this functionality is isolated to ILI251x. > > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> > Cc: Joe Hung <joe_hung@ilitek.com> > Cc: Luca Hsu <luca_hsu@ilitek.com> > --- > V2: New patch > --- > drivers/input/touchscreen/ili210x.c | 31 +++++++++++++++++++++++++++++ > 1 file changed, 31 insertions(+) > > diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c > index 30576a5f2f04..c46553ecabe6 100644 > --- a/drivers/input/touchscreen/ili210x.c > +++ b/drivers/input/touchscreen/ili210x.c > @@ -323,6 +323,34 @@ static irqreturn_t ili210x_irq(int irq, void *irq_data) > return IRQ_HANDLED; > } > > +static int ili251x_firmware_update_resolution(struct device *dev) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct ili210x *priv = i2c_get_clientdata(client); > + u16 resx, resy; > + u8 rs[10]; > + int ret; > + > + /* The firmware update blob might have changed the resolution. */ > + ret = priv->chip->read_reg(client, REG_PANEL_INFO, &rs, sizeof(rs)); > + if (ret) > + return ret; > + > + resx = (rs[1] << 8U) | rs[0]; > + resy = (rs[3] << 8U) | rs[2]; Why do we need the 'U' specifier here? Actually, let's use le16_to_cpup() or get_unaligned_le16(). > + > + /* The value reported by the firmware is invalid. */ > + if (!resx || resx == 0xffff || !resy || resy == 0xfff) Not 0xffff for resy? > + return -EINVAL; > + > + priv->input->absinfo[ABS_X].maximum = resx - 1; > + priv->input->absinfo[ABS_Y].maximum = resy - 1; > + priv->input->absinfo[ABS_MT_POSITION_X].maximum = resx - 1; > + priv->input->absinfo[ABS_MT_POSITION_Y].maximum = resy - 1; There is input_set_abs_max(priv->input, ABS_X, resx - 1); > + > + return 0; > +} > + > static ssize_t ili210x_calibrate(struct device *dev, > struct device_attribute *attr, > const char *buf, size_t count) > @@ -449,6 +477,9 @@ static int ili210x_i2c_probe(struct i2c_client *client, > input_set_abs_params(input, ABS_MT_POSITION_Y, 0, max_xy, 0, 0); > if (priv->chip->has_pressure_reg) > input_set_abs_params(input, ABS_MT_PRESSURE, 0, 0xa, 0, 0); > + /* ILI251x does report valid resolution information, try it. */ > + if (priv->chip == &ili251x_chip) > + ili251x_firmware_update_resolution(dev); Would prefer a flag in chip features vs matching on chip. > touchscreen_parse_properties(input, true, &priv->prop); > > error = input_mt_init_slots(input, priv->chip->max_touches, > -- > 2.32.0 > Thanks.
On 8/30/21 10:55 PM, Dmitry Torokhov wrote: [...] >> + return -EINVAL; >> + >> + priv->input->absinfo[ABS_X].maximum = resx - 1; >> + priv->input->absinfo[ABS_Y].maximum = resy - 1; >> + priv->input->absinfo[ABS_MT_POSITION_X].maximum = resx - 1; >> + priv->input->absinfo[ABS_MT_POSITION_Y].maximum = resy - 1; > > There is > > input_set_abs_max(priv->input, ABS_X, resx - 1); git grep finds nothing in linux-next / your tree on k.org / patchwork. Where is that ? [...]
On Tue, Aug 31, 2021 at 12:49:49AM +0200, Marek Vasut wrote: > On 8/30/21 10:55 PM, Dmitry Torokhov wrote: > > [...] > > > > + return -EINVAL; > > > + > > > + priv->input->absinfo[ABS_X].maximum = resx - 1; > > > + priv->input->absinfo[ABS_Y].maximum = resy - 1; > > > + priv->input->absinfo[ABS_MT_POSITION_X].maximum = resx - 1; > > > + priv->input->absinfo[ABS_MT_POSITION_Y].maximum = resy - 1; > > > > There is > > > > input_set_abs_max(priv->input, ABS_X, resx - 1); > > git grep finds nothing in linux-next / your tree on k.org / patchwork. Where > is that ? Look for INPUT_GENERATE_ABS_ACCESSORS in include/linux/input.h Thanks.
On 8/31/21 1:21 AM, Dmitry Torokhov wrote: > On Tue, Aug 31, 2021 at 12:49:49AM +0200, Marek Vasut wrote: >> On 8/30/21 10:55 PM, Dmitry Torokhov wrote: >> >> [...] >> >>>> + return -EINVAL; >>>> + >>>> + priv->input->absinfo[ABS_X].maximum = resx - 1; >>>> + priv->input->absinfo[ABS_Y].maximum = resy - 1; >>>> + priv->input->absinfo[ABS_MT_POSITION_X].maximum = resx - 1; >>>> + priv->input->absinfo[ABS_MT_POSITION_Y].maximum = resy - 1; >>> >>> There is >>> >>> input_set_abs_max(priv->input, ABS_X, resx - 1); >> >> git grep finds nothing in linux-next / your tree on k.org / patchwork. Where >> is that ? > > Look for INPUT_GENERATE_ABS_ACCESSORS in include/linux/input.h Oh, input_abs_set_max, thanks.
On Tue, Aug 31, 2021 at 01:31:06AM +0200, Marek Vasut wrote: > On 8/31/21 1:21 AM, Dmitry Torokhov wrote: > > On Tue, Aug 31, 2021 at 12:49:49AM +0200, Marek Vasut wrote: > > > On 8/30/21 10:55 PM, Dmitry Torokhov wrote: > > > > > > [...] > > > > > > > > + return -EINVAL; > > > > > + > > > > > + priv->input->absinfo[ABS_X].maximum = resx - 1; > > > > > + priv->input->absinfo[ABS_Y].maximum = resy - 1; > > > > > + priv->input->absinfo[ABS_MT_POSITION_X].maximum = resx - 1; > > > > > + priv->input->absinfo[ABS_MT_POSITION_Y].maximum = resy - 1; > > > > > > > > There is > > > > > > > > input_set_abs_max(priv->input, ABS_X, resx - 1); > > > > > > git grep finds nothing in linux-next / your tree on k.org / patchwork. Where > > > is that ? > > > > Look for INPUT_GENERATE_ABS_ACCESSORS in include/linux/input.h > > Oh, input_abs_set_max, thanks. Ah, sorry, mistyped the name first time around.
On 8/31/21 1:46 AM, Dmitry Torokhov wrote: > On Tue, Aug 31, 2021 at 01:31:06AM +0200, Marek Vasut wrote: >> On 8/31/21 1:21 AM, Dmitry Torokhov wrote: >>> On Tue, Aug 31, 2021 at 12:49:49AM +0200, Marek Vasut wrote: >>>> On 8/30/21 10:55 PM, Dmitry Torokhov wrote: >>>> >>>> [...] >>>> >>>>>> + return -EINVAL; >>>>>> + >>>>>> + priv->input->absinfo[ABS_X].maximum = resx - 1; >>>>>> + priv->input->absinfo[ABS_Y].maximum = resy - 1; >>>>>> + priv->input->absinfo[ABS_MT_POSITION_X].maximum = resx - 1; >>>>>> + priv->input->absinfo[ABS_MT_POSITION_Y].maximum = resy - 1; >>>>> >>>>> There is >>>>> >>>>> input_set_abs_max(priv->input, ABS_X, resx - 1); >>>> >>>> git grep finds nothing in linux-next / your tree on k.org / patchwork. Where >>>> is that ? >>> >>> Look for INPUT_GENERATE_ABS_ACCESSORS in include/linux/input.h >> >> Oh, input_abs_set_max, thanks. > > Ah, sorry, mistyped the name first time around. Yep, no worries, this will be fixed in V3.
diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c index 30576a5f2f04..c46553ecabe6 100644 --- a/drivers/input/touchscreen/ili210x.c +++ b/drivers/input/touchscreen/ili210x.c @@ -323,6 +323,34 @@ static irqreturn_t ili210x_irq(int irq, void *irq_data) return IRQ_HANDLED; } +static int ili251x_firmware_update_resolution(struct device *dev) +{ + struct i2c_client *client = to_i2c_client(dev); + struct ili210x *priv = i2c_get_clientdata(client); + u16 resx, resy; + u8 rs[10]; + int ret; + + /* The firmware update blob might have changed the resolution. */ + ret = priv->chip->read_reg(client, REG_PANEL_INFO, &rs, sizeof(rs)); + if (ret) + return ret; + + resx = (rs[1] << 8U) | rs[0]; + resy = (rs[3] << 8U) | rs[2]; + + /* The value reported by the firmware is invalid. */ + if (!resx || resx == 0xffff || !resy || resy == 0xfff) + return -EINVAL; + + priv->input->absinfo[ABS_X].maximum = resx - 1; + priv->input->absinfo[ABS_Y].maximum = resy - 1; + priv->input->absinfo[ABS_MT_POSITION_X].maximum = resx - 1; + priv->input->absinfo[ABS_MT_POSITION_Y].maximum = resy - 1; + + return 0; +} + static ssize_t ili210x_calibrate(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) @@ -449,6 +477,9 @@ static int ili210x_i2c_probe(struct i2c_client *client, input_set_abs_params(input, ABS_MT_POSITION_Y, 0, max_xy, 0, 0); if (priv->chip->has_pressure_reg) input_set_abs_params(input, ABS_MT_PRESSURE, 0, 0xa, 0, 0); + /* ILI251x does report valid resolution information, try it. */ + if (priv->chip == &ili251x_chip) + ili251x_firmware_update_resolution(dev); touchscreen_parse_properties(input, true, &priv->prop); error = input_mt_init_slots(input, priv->chip->max_touches,
The ili251x firmware protocol permits readout of panel resolution, implement this, but make it possible to override this value using DT bindings. This way, older DTs which contain touchscreen-size-x and touchscreen-size-y properties will behave just like before and new DTs may avoid specifying these for ILI251x. Note that the command format is different on other controllers, so this functionality is isolated to ILI251x. Signed-off-by: Marek Vasut <marex@denx.de> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> Cc: Joe Hung <joe_hung@ilitek.com> Cc: Luca Hsu <luca_hsu@ilitek.com> --- V2: New patch --- drivers/input/touchscreen/ili210x.c | 31 +++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)