Re: [PATCH] drm/bridge/sii8620: add remote control support
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 }, /* Forward */
>>> + [0
Re: [PATCH] drm/bridge/sii8620: add remote control support
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[] = {
>> 0x39, /* Main System */
>>
Re: [PATCH] drm/bridge/sii8620: add remote control support
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,
struct si
Re: [PATCH] drm/bridge/sii8620: add remote control support
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 },
> > + [0x75] = { KEY_F5 },
Re: [PATCH] drm/bridge/sii8620: add remote control support
Hi Maciej,
This is my first time looking at anything input related, so pardon if
I'm off the mark here.
On 3 August 2017 at 08:44, Maciej Purski wrote:
[...]
> +static struct {
> + u16 key;
> + u16 extra_key;
> + bool autorepeat;
> +} rcp_keymap[] = {
Ideally this would be "const" allowing the compiler to move the data
to the .rodata section, making exploits a bit harder.
Then again struct input_dev::keycode is "void *" so that cannot quite work atm.
One could(?) toggle make that a const, updating the users. Many of
them memcpy from const data into keycode, while others k[z]alloc and
them memcpy.
Might be worth listing these in the input-tree TODO list ;-)
[...]
> + set_bit(EV_KEY, i_dev->evbit);
> + i_dev->name = "MHL Remote Control";
> + i_dev->keycode = rcp_keymap;
> + i_dev->keycodesize = sizeof(u16);
According to the docs "... keycodesize the size of each entry in it
(in bytes)...".
As such this should be sizeof(rcp_keymap[0])
At the same time, the code in input.c has a hidden assumption -
keycode is an array of u8, u16 or u32 values.
See functions input_fetch_keycode() and input_default_setkeycode() for details.
In this patch, the size is 6 and everything will go crazy.
Couple ideas come to mind:
- enforce keycode layout - must be a simple u8/16/32 array.
- add separate field for the size of "key", such that one can have
more complex data stored in keycode.
Of course, I could be completely wrong :-)
HTH
Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge/sii8620: add remote control support
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)
> +{
> + input_report_key(ctx->rcp_
[PATCH] drm/bridge/sii8620: add remote control support
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)
+{
+ input_report_key(ctx->rcp_input_dev,
+ rcp_keymap[keycode].key, pressed);
+
+ if (rcp_keymap[keycode].extra_key)
+ input_report_key(ctx->rcp_input_dev,
+ rcp_keymap[keycode].extra_key, pressed);
+}
+
+static bool sii8620_rcp_consume(struct sii8620 *ctx, u8 keycode)
+{
+ bool pressed = !(keycode & MHL_RCP_KEY_RELEASED_MASK);
+
+ if (!ctx->rcp_input_dev) {
+ dev_dbg(ctx->dev, "RCP input device not initialized\n");
+ return false;
+ }
+
+ keycode &= MHL_RCP_KEY_ID_MASK;
+ if (keycode >= ARRAY_S
