Re: [PATCH] drm/bridge/sii8620: add remote control support

2017-08-23 Thread Hans Verkuil
On 08/16/2017 01:55 PM, Andrzej Hajda wrote:
> On 03.08.2017 10:28, Hans Verkuil wrote:
>> Hi Maciej,
>>
>> Unfortunately I do not have the MHL spec, but I was wondering what the
>> relationship between RCP and CEC is. CEC has remote control support as
>> well, so is RCP that subset of the CEC specification or is it completely
>> separate?
> 
> We also do not have MHL specs. From my research it looks like MHL
> consortium was mainly focused on supporting different input devices -
> remote control, mice, keyboard, touchscreen, game controller, etc. In
> public data sheets of some chips Lattice/Silicon Image (main MHL chip
> producer) suggest they do not support CEC pass-through via MHL[1].
> On the other side superMHL extends RCP with support for multiple devices
> [2], so for me it looks like RCP wants to be an alternative to CEC.
> But all this is just my interpretation of info found on the Net.

Surely Samsung must have access to the MHL specs? (I know, big company,
hard to figure out who to talk to)

I ask because the devil is often in the details. I recently fixed the CEC
auto-repeat implementation that was subtly wrong. It is very desirable
if this driver was based on the actual spec. You should definitely test
auto-repeat.

I gather that the scancodes for keys in MHL are identical to those of CEC?
Since in your v3 patch you reuse those.

Regards,

Hans

> 
> [1]:
> http://www.latticesemi.com/~/media/LatticeSemi/Documents/DataSheets/ASSP/SiI-DS-1128_Public.pdf?document_id=51627
> [2]: https://en.wikipedia.org/wiki/Mobile_High-Definition_Link#superMHL
> 
> Regards
> Andrzej
> 
>>
>> I'm CC-ing Sean Young and the linux-media mailinglist as well since Sean
>> maintains the rc subsystem. Which you probably should use, but I'm not the
>> expert on that.
>>
>> Regards,
>>
>>  Hans
>>
>> On 08/03/17 09:44, Maciej Purski wrote:
>>> MHL specification defines Remote Control Protocol(RCP) to
>>> send input events between MHL devices.
>>> The driver now recognizes RCP messages and reacts to them
>>> by reporting key events to input subsystem, allowing
>>> a user to control a device using TV remote control.
>>>
>>> Signed-off-by: Maciej Purski 
>>> ---
>>>  drivers/gpu/drm/bridge/sil-sii8620.c | 188 
>>> ++-
>>>  include/drm/bridge/mhl.h |   4 +
>>>  2 files changed, 187 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c 
>>> b/drivers/gpu/drm/bridge/sil-sii8620.c
>>> index 2d51a22..7e75f2f 100644
>>> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
>>> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
>>> @@ -19,6 +19,7 @@
>>>  #include 
>>>  #include 
>>>  #include 
>>> +#include 
>>>  #include 
>>>  #include 
>>>  #include 
>>> @@ -58,6 +59,7 @@ enum sii8620_mt_state {
>>>  struct sii8620 {
>>> struct drm_bridge bridge;
>>> struct device *dev;
>>> +   struct input_dev *rcp_input_dev;
>>> struct clk *clk_xtal;
>>> struct gpio_desc *gpio_reset;
>>> struct gpio_desc *gpio_int;
>>> @@ -106,6 +108,82 @@ struct sii8620_mt_msg {
>>> sii8620_cb continuation;
>>>  };
>>>  
>>> +static struct {
>>> +   u16 key;
>>> +   u16 extra_key;
>>> +   bool autorepeat;
>>> +}  rcp_keymap[] = {
>>> +   [0x00] = { KEY_SELECT },
>>> +   [0x01] = { KEY_UP, 0, true },
>>> +   [0x02] = { KEY_DOWN, 0, true },
>>> +   [0x03] = { KEY_LEFT, 0, true },
>>> +   [0x04] = { KEY_RIGHT, 0, true },
>>> +
>>> +   [0x05] = { KEY_RIGHT, KEY_UP, true },
>>> +   [0x06] = { KEY_RIGHT, KEY_DOWN, true },
>>> +   [0x07] = { KEY_LEFT,  KEY_UP, true },
>>> +   [0x08] = { KEY_LEFT,  KEY_DOWN, true },
>>> +
>>> +   [0x09] = { KEY_MENU },
>>> +   [0x0A] = { KEY_UNKNOWN },
>>> +   [0x0B] = { KEY_UNKNOWN },
>>> +   [0x0C] = { KEY_BOOKMARKS },
>>> +   [0x0D] = { KEY_EXIT },
>>> +
>>> +   [0x20] = { KEY_NUMERIC_0 },
>>> +   [0x21] = { KEY_NUMERIC_1 },
>>> +   [0x22] = { KEY_NUMERIC_2 },
>>> +   [0x23] = { KEY_NUMERIC_3 },
>>> +   [0x24] = { KEY_NUMERIC_4 },
>>> +   [0x25] = { KEY_NUMERIC_5 },
>>> +   [0x26] = { KEY_NUMERIC_6 },
>>> +   [0x27] = { KEY_NUMERIC_7 },
>>> +   [0x28] = { KEY_NUMERIC_8 },
>>> +   [0x29] = { KEY_NUMERIC_9 },
>>> +
>>> +   [0x2A] = { KEY_DOT },
>>> +   [0x2B] = { KEY_ENTER },
>>> +   [0x2C] = { KEY_CLEAR },
>>> +
>>> +   [0x30] = { KEY_CHANNELUP, 0, true },
>>> +   [0x31] = { KEY_CHANNELDOWN, 0, true },
>>> +
>>> +   [0x33] = { KEY_SOUND },
>>> +   [0x35] = { KEY_PROGRAM }, /* Show Information */
>>> +
>>> +   [0x37] = { KEY_PAGEUP, 0, true },
>>> +   [0x38] = { KEY_PAGEDOWN, 0, true },
>>> +
>>> +   [0x41] = { KEY_VOLUMEUP, 0, true },
>>> +   [0x42] = { KEY_VOLUMEDOWN, 0, true },
>>> +   [0x43] = { KEY_MUTE },
>>> +   [0x44] = { KEY_PLAY },
>>> +   [0x45] = { KEY_STOP },
>>> +   [0x46] = { KEY_PLAYPAUSE }, /* Pause */
>>> +   [0x47] = { KEY_RECORD },
>>> +   [0x48] = { KEY_REWIND, 0, true },
>>> +   [0x49] = { KEY_FASTFORWARD, 0, true },
>>> +   [0x4A] = { KEY_EJECTCD },
>>> +   [0x4B] = { KEY_NEXTSONG, 0, true }, 

Re: [PATCH] drm/bridge/sii8620: add remote control support

2017-08-16 Thread Andrzej Hajda
On 03.08.2017 10:28, Hans Verkuil wrote:
> Hi Maciej,
>
> Unfortunately I do not have the MHL spec, but I was wondering what the
> relationship between RCP and CEC is. CEC has remote control support as
> well, so is RCP that subset of the CEC specification or is it completely
> separate?

We also do not have MHL specs. From my research it looks like MHL
consortium was mainly focused on supporting different input devices -
remote control, mice, keyboard, touchscreen, game controller, etc. In
public data sheets of some chips Lattice/Silicon Image (main MHL chip
producer) suggest they do not support CEC pass-through via MHL[1].
On the other side superMHL extends RCP with support for multiple devices
[2], so for me it looks like RCP wants to be an alternative to CEC.
But all this is just my interpretation of info found on the Net.

[1]:
http://www.latticesemi.com/~/media/LatticeSemi/Documents/DataSheets/ASSP/SiI-DS-1128_Public.pdf?document_id=51627
[2]: https://en.wikipedia.org/wiki/Mobile_High-Definition_Link#superMHL

Regards
Andrzej

>
> I'm CC-ing Sean Young and the linux-media mailinglist as well since Sean
> maintains the rc subsystem. Which you probably should use, but I'm not the
> expert on that.
>
> Regards,
>
>   Hans
>
> On 08/03/17 09:44, Maciej Purski wrote:
>> MHL specification defines Remote Control Protocol(RCP) to
>> send input events between MHL devices.
>> The driver now recognizes RCP messages and reacts to them
>> by reporting key events to input subsystem, allowing
>> a user to control a device using TV remote control.
>>
>> Signed-off-by: Maciej Purski 
>> ---
>>  drivers/gpu/drm/bridge/sil-sii8620.c | 188 
>> ++-
>>  include/drm/bridge/mhl.h |   4 +
>>  2 files changed, 187 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c 
>> b/drivers/gpu/drm/bridge/sil-sii8620.c
>> index 2d51a22..7e75f2f 100644
>> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
>> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
>> @@ -19,6 +19,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -58,6 +59,7 @@ enum sii8620_mt_state {
>>  struct sii8620 {
>>  struct drm_bridge bridge;
>>  struct device *dev;
>> +struct input_dev *rcp_input_dev;
>>  struct clk *clk_xtal;
>>  struct gpio_desc *gpio_reset;
>>  struct gpio_desc *gpio_int;
>> @@ -106,6 +108,82 @@ struct sii8620_mt_msg {
>>  sii8620_cb continuation;
>>  };
>>  
>> +static struct {
>> +u16 key;
>> +u16 extra_key;
>> +bool autorepeat;
>> +}  rcp_keymap[] = {
>> +[0x00] = { KEY_SELECT },
>> +[0x01] = { KEY_UP, 0, true },
>> +[0x02] = { KEY_DOWN, 0, true },
>> +[0x03] = { KEY_LEFT, 0, true },
>> +[0x04] = { KEY_RIGHT, 0, true },
>> +
>> +[0x05] = { KEY_RIGHT, KEY_UP, true },
>> +[0x06] = { KEY_RIGHT, KEY_DOWN, true },
>> +[0x07] = { KEY_LEFT,  KEY_UP, true },
>> +[0x08] = { KEY_LEFT,  KEY_DOWN, true },
>> +
>> +[0x09] = { KEY_MENU },
>> +[0x0A] = { KEY_UNKNOWN },
>> +[0x0B] = { KEY_UNKNOWN },
>> +[0x0C] = { KEY_BOOKMARKS },
>> +[0x0D] = { KEY_EXIT },
>> +
>> +[0x20] = { KEY_NUMERIC_0 },
>> +[0x21] = { KEY_NUMERIC_1 },
>> +[0x22] = { KEY_NUMERIC_2 },
>> +[0x23] = { KEY_NUMERIC_3 },
>> +[0x24] = { KEY_NUMERIC_4 },
>> +[0x25] = { KEY_NUMERIC_5 },
>> +[0x26] = { KEY_NUMERIC_6 },
>> +[0x27] = { KEY_NUMERIC_7 },
>> +[0x28] = { KEY_NUMERIC_8 },
>> +[0x29] = { KEY_NUMERIC_9 },
>> +
>> +[0x2A] = { KEY_DOT },
>> +[0x2B] = { KEY_ENTER },
>> +[0x2C] = { KEY_CLEAR },
>> +
>> +[0x30] = { KEY_CHANNELUP, 0, true },
>> +[0x31] = { KEY_CHANNELDOWN, 0, true },
>> +
>> +[0x33] = { KEY_SOUND },
>> +[0x35] = { KEY_PROGRAM }, /* Show Information */
>> +
>> +[0x37] = { KEY_PAGEUP, 0, true },
>> +[0x38] = { KEY_PAGEDOWN, 0, true },
>> +
>> +[0x41] = { KEY_VOLUMEUP, 0, true },
>> +[0x42] = { KEY_VOLUMEDOWN, 0, true },
>> +[0x43] = { KEY_MUTE },
>> +[0x44] = { KEY_PLAY },
>> +[0x45] = { KEY_STOP },
>> +[0x46] = { KEY_PLAYPAUSE }, /* Pause */
>> +[0x47] = { KEY_RECORD },
>> +[0x48] = { KEY_REWIND, 0, true },
>> +[0x49] = { KEY_FASTFORWARD, 0, true },
>> +[0x4A] = { KEY_EJECTCD },
>> +[0x4B] = { KEY_NEXTSONG, 0, true }, /* Forward */
>> +[0x4C] = { KEY_PREVIOUSSONG, 0, true }, /* Backward */
>> +
>> +[0x60] = { KEY_PLAYPAUSE }, /* Play */
>> +[0x61] = { KEY_PLAYPAUSE }, /* Pause the Play */
>> +[0x62] = { KEY_RECORD },
>> +[0x63] = { KEY_PAUSE },
>> +[0x64] = { KEY_STOP },
>> +[0x65] = { KEY_MUTE },
>> +[0x66] = { KEY_MUTE }, /* Restore Mute */
>> +
>> +[0x71] = { KEY_F1 },
>> +[0x72] = { KEY_F2 },
>> +[0x73] = { KEY_F3 },
>> +[0x74] = { KEY_F4 },
>> +[0x75] = { KEY_F5 },
>> +
>> +[0x7E] = { KEY_VENDOR },
>> +};
>> +
>>  static const u8 sii8620_i2c_page[] = {
>>  

Re: [PATCH] drm/bridge/sii8620: add remote control support

2017-08-10 Thread Maciej Purski

Hi Hans,

Thank you for your reply.  All dongles that we used to work with in Samsung
convert CEC protocol to RCP which we get through MHL Sideband Channel.
All we can do in the driver is to report RCP messages to user-space.

On the RC subsystem: your point is right, which was confirmed
by Sean and I'm going to use RC subsystem there.

Regards,

Maciej Purski

On 08/03/2017 10:28 AM, Hans Verkuil wrote:


Hi Maciej,

Unfortunately I do not have the MHL spec, but I was wondering what the
relationship between RCP and CEC is. CEC has remote control support as
well, so is RCP that subset of the CEC specification or is it completely
separate?

I'm CC-ing Sean Young and the linux-media mailinglist as well since Sean
maintains the rc subsystem. Which you probably should use, but I'm not the
expert on that.

Regards,

Hans

On 08/03/17 09:44, Maciej Purski wrote:

MHL specification defines Remote Control Protocol(RCP) to
send input events between MHL devices.
The driver now recognizes RCP messages and reacts to them
by reporting key events to input subsystem, allowing
a user to control a device using TV remote control.

Signed-off-by: Maciej Purski 
---
  drivers/gpu/drm/bridge/sil-sii8620.c | 188 ++-
  include/drm/bridge/mhl.h |   4 +
  2 files changed, 187 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c 
b/drivers/gpu/drm/bridge/sil-sii8620.c
index 2d51a22..7e75f2f 100644
--- a/drivers/gpu/drm/bridge/sil-sii8620.c
+++ b/drivers/gpu/drm/bridge/sil-sii8620.c
@@ -19,6 +19,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -58,6 +59,7 @@ enum sii8620_mt_state {
  struct sii8620 {
struct drm_bridge bridge;
struct device *dev;
+   struct input_dev *rcp_input_dev;
struct clk *clk_xtal;
struct gpio_desc *gpio_reset;
struct gpio_desc *gpio_int;
@@ -106,6 +108,82 @@ struct sii8620_mt_msg {
sii8620_cb continuation;
  };
  
+static struct {

+   u16 key;
+   u16 extra_key;
+   bool autorepeat;
+}  rcp_keymap[] = {
+   [0x00] = { KEY_SELECT },
+   [0x01] = { KEY_UP, 0, true },
+   [0x02] = { KEY_DOWN, 0, true },
+   [0x03] = { KEY_LEFT, 0, true },
+   [0x04] = { KEY_RIGHT, 0, true },
+
+   [0x05] = { KEY_RIGHT, KEY_UP, true },
+   [0x06] = { KEY_RIGHT, KEY_DOWN, true },
+   [0x07] = { KEY_LEFT,  KEY_UP, true },
+   [0x08] = { KEY_LEFT,  KEY_DOWN, true },
+
+   [0x09] = { KEY_MENU },
+   [0x0A] = { KEY_UNKNOWN },
+   [0x0B] = { KEY_UNKNOWN },
+   [0x0C] = { KEY_BOOKMARKS },
+   [0x0D] = { KEY_EXIT },
+
+   [0x20] = { KEY_NUMERIC_0 },
+   [0x21] = { KEY_NUMERIC_1 },
+   [0x22] = { KEY_NUMERIC_2 },
+   [0x23] = { KEY_NUMERIC_3 },
+   [0x24] = { KEY_NUMERIC_4 },
+   [0x25] = { KEY_NUMERIC_5 },
+   [0x26] = { KEY_NUMERIC_6 },
+   [0x27] = { KEY_NUMERIC_7 },
+   [0x28] = { KEY_NUMERIC_8 },
+   [0x29] = { KEY_NUMERIC_9 },
+
+   [0x2A] = { KEY_DOT },
+   [0x2B] = { KEY_ENTER },
+   [0x2C] = { KEY_CLEAR },
+
+   [0x30] = { KEY_CHANNELUP, 0, true },
+   [0x31] = { KEY_CHANNELDOWN, 0, true },
+
+   [0x33] = { KEY_SOUND },
+   [0x35] = { KEY_PROGRAM }, /* Show Information */
+
+   [0x37] = { KEY_PAGEUP, 0, true },
+   [0x38] = { KEY_PAGEDOWN, 0, true },
+
+   [0x41] = { KEY_VOLUMEUP, 0, true },
+   [0x42] = { KEY_VOLUMEDOWN, 0, true },
+   [0x43] = { KEY_MUTE },
+   [0x44] = { KEY_PLAY },
+   [0x45] = { KEY_STOP },
+   [0x46] = { KEY_PLAYPAUSE }, /* Pause */
+   [0x47] = { KEY_RECORD },
+   [0x48] = { KEY_REWIND, 0, true },
+   [0x49] = { KEY_FASTFORWARD, 0, true },
+   [0x4A] = { KEY_EJECTCD },
+   [0x4B] = { KEY_NEXTSONG, 0, true }, /* Forward */
+   [0x4C] = { KEY_PREVIOUSSONG, 0, true }, /* Backward */
+
+   [0x60] = { KEY_PLAYPAUSE }, /* Play */
+   [0x61] = { KEY_PLAYPAUSE }, /* Pause the Play */
+   [0x62] = { KEY_RECORD },
+   [0x63] = { KEY_PAUSE },
+   [0x64] = { KEY_STOP },
+   [0x65] = { KEY_MUTE },
+   [0x66] = { KEY_MUTE }, /* Restore Mute */
+
+   [0x71] = { KEY_F1 },
+   [0x72] = { KEY_F2 },
+   [0x73] = { KEY_F3 },
+   [0x74] = { KEY_F4 },
+   [0x75] = { KEY_F5 },
+
+   [0x7E] = { KEY_VENDOR },
+};
+
  static const u8 sii8620_i2c_page[] = {
0x39, /* Main System */
0x3d, /* TDM and HSIC */
@@ -431,6 +509,16 @@ static void sii8620_mt_rap(struct sii8620 *ctx, u8 code)
sii8620_mt_msc_msg(ctx, MHL_MSC_MSG_RAP, code);
  }
  
+static void sii8620_mt_rcpk(struct sii8620 *ctx, u8 code)

+{
+   sii8620_mt_msc_msg(ctx, MHL_MSC_MSG_RCPK, code);
+}
+
+static void sii8620_mt_rcpe(struct sii8620 *ctx, u8 code)
+{
+   sii8620_mt_msc_msg(ctx, MHL_MSC_MSG_RCPE, code);
+}
+
  static void sii8620_mt_read_devcap_send(struct sii8620 *ctx,
   

Re: [PATCH] drm/bridge/sii8620: add remote control support

2017-08-06 Thread Sean Young
Hi Maciej,

On Thu, Aug 03, 2017 at 10:28:04AM +0200, Hans Verkuil wrote:
> Hi Maciej,
> 
> Unfortunately I do not have the MHL spec, but I was wondering what the
> relationship between RCP and CEC is. CEC has remote control support as
> well, so is RCP that subset of the CEC specification or is it completely
> separate?
> 
> I'm CC-ing Sean Young and the linux-media mailinglist as well since Sean
> maintains the rc subsystem. Which you probably should use, but I'm not the
> expert on that.

Yes, I agree. Using rc core the keymap can be changed from user space, and
is defined elsewhere. 

In include/media/rc-map.h you'll have to define a new protocol (unless the
protocol is CEC).

In drivers/media/rc/keymaps/ a new keymap should be defined.

Use devm_rc_allocate_device(RC_DRIVER_SCANCODE) to create the device; then
you'll need to at least set these members:

input_phys = DRIVER_NAME "/input0";
input_id.bustype = BUS_HOST;
map_name = RC_MAP_RCP;
allowed_protocols = RC_BIT_RCP;
driver_name = DRIVER_NAME;
device_name = DEVICE_NAME;

Then register with devm_rc_register_device(), and report keys with 
rc_keydown() and rc_keyup().

> On 08/03/17 09:44, Maciej Purski wrote:
> > MHL specification defines Remote Control Protocol(RCP) to
> > send input events between MHL devices.
> > The driver now recognizes RCP messages and reacts to them
> > by reporting key events to input subsystem, allowing
> > a user to control a device using TV remote control.
> > 
> > Signed-off-by: Maciej Purski 
> > ---
> >  drivers/gpu/drm/bridge/sil-sii8620.c | 188 
> > ++-
> >  include/drm/bridge/mhl.h |   4 +
> >  2 files changed, 187 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c 
> > b/drivers/gpu/drm/bridge/sil-sii8620.c
> > index 2d51a22..7e75f2f 100644
> > --- a/drivers/gpu/drm/bridge/sil-sii8620.c
> > +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
> > @@ -19,6 +19,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -58,6 +59,7 @@ enum sii8620_mt_state {
> >  struct sii8620 {
> > struct drm_bridge bridge;
> > struct device *dev;
> > +   struct input_dev *rcp_input_dev;
> > struct clk *clk_xtal;
> > struct gpio_desc *gpio_reset;
> > struct gpio_desc *gpio_int;
> > @@ -106,6 +108,82 @@ struct sii8620_mt_msg {
> > sii8620_cb continuation;
> >  };
> >  
> > +static struct {
> > +   u16 key;
> > +   u16 extra_key;
> > +   bool autorepeat;

Again let the input layer handle autorepeat, see REP_DELAY and REP_PERIOD.

> > +}  rcp_keymap[] = {
> > +   [0x00] = { KEY_SELECT },
> > +   [0x01] = { KEY_UP, 0, true },
> > +   [0x02] = { KEY_DOWN, 0, true },
> > +   [0x03] = { KEY_LEFT, 0, true },
> > +   [0x04] = { KEY_RIGHT, 0, true },
> > +
> > +   [0x05] = { KEY_RIGHT, KEY_UP, true },
> > +   [0x06] = { KEY_RIGHT, KEY_DOWN, true },
> > +   [0x07] = { KEY_LEFT,  KEY_UP, true },
> > +   [0x08] = { KEY_LEFT,  KEY_DOWN, true },

This should be replaced with KEY_RIGHT_UP, RC_RIGHT_DOWN, etc.

> > +
> > +   [0x09] = { KEY_MENU },
> > +   [0x0A] = { KEY_UNKNOWN },
> > +   [0x0B] = { KEY_UNKNOWN },
> > +   [0x0C] = { KEY_BOOKMARKS },
> > +   [0x0D] = { KEY_EXIT },
> > +
> > +   [0x20] = { KEY_NUMERIC_0 },
> > +   [0x21] = { KEY_NUMERIC_1 },
> > +   [0x22] = { KEY_NUMERIC_2 },
> > +   [0x23] = { KEY_NUMERIC_3 },
> > +   [0x24] = { KEY_NUMERIC_4 },
> > +   [0x25] = { KEY_NUMERIC_5 },
> > +   [0x26] = { KEY_NUMERIC_6 },
> > +   [0x27] = { KEY_NUMERIC_7 },
> > +   [0x28] = { KEY_NUMERIC_8 },
> > +   [0x29] = { KEY_NUMERIC_9 },
> > +
> > +   [0x2A] = { KEY_DOT },
> > +   [0x2B] = { KEY_ENTER },
> > +   [0x2C] = { KEY_CLEAR },
> > +
> > +   [0x30] = { KEY_CHANNELUP, 0, true },
> > +   [0x31] = { KEY_CHANNELDOWN, 0, true },
> > +
> > +   [0x33] = { KEY_SOUND },
> > +   [0x35] = { KEY_PROGRAM }, /* Show Information */
> > +
> > +   [0x37] = { KEY_PAGEUP, 0, true },
> > +   [0x38] = { KEY_PAGEDOWN, 0, true },
> > +
> > +   [0x41] = { KEY_VOLUMEUP, 0, true },
> > +   [0x42] = { KEY_VOLUMEDOWN, 0, true },
> > +   [0x43] = { KEY_MUTE },
> > +   [0x44] = { KEY_PLAY },
> > +   [0x45] = { KEY_STOP },
> > +   [0x46] = { KEY_PLAYPAUSE }, /* Pause */
> > +   [0x47] = { KEY_RECORD },
> > +   [0x48] = { KEY_REWIND, 0, true },
> > +   [0x49] = { KEY_FASTFORWARD, 0, true },
> > +   [0x4A] = { KEY_EJECTCD },
> > +   [0x4B] = { KEY_NEXTSONG, 0, true }, /* Forward */
> > +   [0x4C] = { KEY_PREVIOUSSONG, 0, true }, /* Backward */
> > +
> > +   [0x60] = { KEY_PLAYPAUSE }, /* Play */
> > +   [0x61] = { KEY_PLAYPAUSE }, /* Pause the Play */
> > +   [0x62] = { KEY_RECORD },
> > +   [0x63] = { KEY_PAUSE },
> > +   [0x64] = { KEY_STOP },
> > +   [0x65] = { KEY_MUTE },
> > +   [0x66] = { KEY_MUTE }, /* Restore Mute */
> > +
> > +   [0x71] = { KEY_F1 },
> > +   [0x72] = { KEY_F2 },
> > +   [0x73] = { KEY_F3 },
> > +   [0x74] = { KEY_F4 },
> > +  

Re: [PATCH] drm/bridge/sii8620: add remote control support

2017-08-03 Thread Hans Verkuil
Hi Maciej,

Unfortunately I do not have the MHL spec, but I was wondering what the
relationship between RCP and CEC is. CEC has remote control support as
well, so is RCP that subset of the CEC specification or is it completely
separate?

I'm CC-ing Sean Young and the linux-media mailinglist as well since Sean
maintains the rc subsystem. Which you probably should use, but I'm not the
expert on that.

Regards,

Hans

On 08/03/17 09:44, Maciej Purski wrote:
> MHL specification defines Remote Control Protocol(RCP) to
> send input events between MHL devices.
> The driver now recognizes RCP messages and reacts to them
> by reporting key events to input subsystem, allowing
> a user to control a device using TV remote control.
> 
> Signed-off-by: Maciej Purski 
> ---
>  drivers/gpu/drm/bridge/sil-sii8620.c | 188 
> ++-
>  include/drm/bridge/mhl.h |   4 +
>  2 files changed, 187 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c 
> b/drivers/gpu/drm/bridge/sil-sii8620.c
> index 2d51a22..7e75f2f 100644
> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
> @@ -19,6 +19,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -58,6 +59,7 @@ enum sii8620_mt_state {
>  struct sii8620 {
>   struct drm_bridge bridge;
>   struct device *dev;
> + struct input_dev *rcp_input_dev;
>   struct clk *clk_xtal;
>   struct gpio_desc *gpio_reset;
>   struct gpio_desc *gpio_int;
> @@ -106,6 +108,82 @@ struct sii8620_mt_msg {
>   sii8620_cb continuation;
>  };
>  
> +static struct {
> + u16 key;
> + u16 extra_key;
> + bool autorepeat;
> +}  rcp_keymap[] = {
> + [0x00] = { KEY_SELECT },
> + [0x01] = { KEY_UP, 0, true },
> + [0x02] = { KEY_DOWN, 0, true },
> + [0x03] = { KEY_LEFT, 0, true },
> + [0x04] = { KEY_RIGHT, 0, true },
> +
> + [0x05] = { KEY_RIGHT, KEY_UP, true },
> + [0x06] = { KEY_RIGHT, KEY_DOWN, true },
> + [0x07] = { KEY_LEFT,  KEY_UP, true },
> + [0x08] = { KEY_LEFT,  KEY_DOWN, true },
> +
> + [0x09] = { KEY_MENU },
> + [0x0A] = { KEY_UNKNOWN },
> + [0x0B] = { KEY_UNKNOWN },
> + [0x0C] = { KEY_BOOKMARKS },
> + [0x0D] = { KEY_EXIT },
> +
> + [0x20] = { KEY_NUMERIC_0 },
> + [0x21] = { KEY_NUMERIC_1 },
> + [0x22] = { KEY_NUMERIC_2 },
> + [0x23] = { KEY_NUMERIC_3 },
> + [0x24] = { KEY_NUMERIC_4 },
> + [0x25] = { KEY_NUMERIC_5 },
> + [0x26] = { KEY_NUMERIC_6 },
> + [0x27] = { KEY_NUMERIC_7 },
> + [0x28] = { KEY_NUMERIC_8 },
> + [0x29] = { KEY_NUMERIC_9 },
> +
> + [0x2A] = { KEY_DOT },
> + [0x2B] = { KEY_ENTER },
> + [0x2C] = { KEY_CLEAR },
> +
> + [0x30] = { KEY_CHANNELUP, 0, true },
> + [0x31] = { KEY_CHANNELDOWN, 0, true },
> +
> + [0x33] = { KEY_SOUND },
> + [0x35] = { KEY_PROGRAM }, /* Show Information */
> +
> + [0x37] = { KEY_PAGEUP, 0, true },
> + [0x38] = { KEY_PAGEDOWN, 0, true },
> +
> + [0x41] = { KEY_VOLUMEUP, 0, true },
> + [0x42] = { KEY_VOLUMEDOWN, 0, true },
> + [0x43] = { KEY_MUTE },
> + [0x44] = { KEY_PLAY },
> + [0x45] = { KEY_STOP },
> + [0x46] = { KEY_PLAYPAUSE }, /* Pause */
> + [0x47] = { KEY_RECORD },
> + [0x48] = { KEY_REWIND, 0, true },
> + [0x49] = { KEY_FASTFORWARD, 0, true },
> + [0x4A] = { KEY_EJECTCD },
> + [0x4B] = { KEY_NEXTSONG, 0, true }, /* Forward */
> + [0x4C] = { KEY_PREVIOUSSONG, 0, true }, /* Backward */
> +
> + [0x60] = { KEY_PLAYPAUSE }, /* Play */
> + [0x61] = { KEY_PLAYPAUSE }, /* Pause the Play */
> + [0x62] = { KEY_RECORD },
> + [0x63] = { KEY_PAUSE },
> + [0x64] = { KEY_STOP },
> + [0x65] = { KEY_MUTE },
> + [0x66] = { KEY_MUTE }, /* Restore Mute */
> +
> + [0x71] = { KEY_F1 },
> + [0x72] = { KEY_F2 },
> + [0x73] = { KEY_F3 },
> + [0x74] = { KEY_F4 },
> + [0x75] = { KEY_F5 },
> +
> + [0x7E] = { KEY_VENDOR },
> +};
> +
>  static const u8 sii8620_i2c_page[] = {
>   0x39, /* Main System */
>   0x3d, /* TDM and HSIC */
> @@ -431,6 +509,16 @@ static void sii8620_mt_rap(struct sii8620 *ctx, u8 code)
>   sii8620_mt_msc_msg(ctx, MHL_MSC_MSG_RAP, code);
>  }
>  
> +static void sii8620_mt_rcpk(struct sii8620 *ctx, u8 code)
> +{
> + sii8620_mt_msc_msg(ctx, MHL_MSC_MSG_RCPK, code);
> +}
> +
> +static void sii8620_mt_rcpe(struct sii8620 *ctx, u8 code)
> +{
> + sii8620_mt_msc_msg(ctx, MHL_MSC_MSG_RCPE, code);
> +}
> +
>  static void sii8620_mt_read_devcap_send(struct sii8620 *ctx,
>   struct sii8620_mt_msg *msg)
>  {
> @@ -1753,6 +1841,43 @@ static void sii8620_send_features(struct sii8620 *ctx)
>   sii8620_write_buf(ctx, REG_MDT_XMIT_WRITE_PORT, buf, ARRAY_SIZE(buf));
>  }
>  
> +static void sii8620_rcp_report_key(struct sii8620 *ctx, u8 keycode, bool 
> pressed)
> +{
> +