diff mbox

[v3] drm/bridge/sii8620: add remote control support

Message ID 1503308001-20661-1-git-send-email-m.purski@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Maciej Purski Aug. 21, 2017, 9:33 a.m. UTC
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.

Changes in v2:
- use RC subsystem (including CEC keymap)
- RC device initialized in attach drm_bridge callback and
  removed in detach callback. This is necessary, because RC_CORE,
  which is needed during rc_dev init, is loaded after sii8620.
  DRM bridge is binded later which solves the problem.
- add RC_CORE dependency

Changes in v3:
- fix error handling in init_rcp and in attach callback

Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
 drivers/gpu/drm/bridge/Kconfig       |   2 +-
 drivers/gpu/drm/bridge/sil-sii8620.c | 101 +++++++++++++++++++++++++++++++++--
 include/drm/bridge/mhl.h             |   4 ++
 3 files changed, 101 insertions(+), 6 deletions(-)

Comments

Hans Verkuil Aug. 23, 2017, 7:03 a.m. UTC | #1
Maciej,

I'm cross-posting this to linux-media and the rc maintainer Sean Young.

Note that various RC defines have been renamed in the upcoming 4.14. It might be
better to base your code on top of https://git.linuxtv.org/media_tree.git/
which has those patches merged.

Regards,

	Hans

On 08/21/2017 11:33 AM, 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.
> 
> Changes in v2:
> - use RC subsystem (including CEC keymap)
> - RC device initialized in attach drm_bridge callback and
>   removed in detach callback. This is necessary, because RC_CORE,
>   which is needed during rc_dev init, is loaded after sii8620.
>   DRM bridge is binded later which solves the problem.
> - add RC_CORE dependency
> 
> Changes in v3:
> - fix error handling in init_rcp and in attach callback
> 
> Signed-off-by: Maciej Purski <m.purski@samsung.com>
> ---
>  drivers/gpu/drm/bridge/Kconfig       |   2 +-
>  drivers/gpu/drm/bridge/sil-sii8620.c | 101 +++++++++++++++++++++++++++++++++--
>  include/drm/bridge/mhl.h             |   4 ++
>  3 files changed, 101 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index adf9ae0..6ef901c 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -71,7 +71,7 @@ config DRM_PARADE_PS8622
>  
>  config DRM_SIL_SII8620
>  	tristate "Silicon Image SII8620 HDMI/MHL bridge"
> -	depends on OF
> +	depends on OF && RC_CORE
>  	select DRM_KMS_HELPER
>  	help
>  	  Silicon Image SII8620 HDMI/MHL bridge chip driver.
> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
> index 2d51a22..e072bca 100644
> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
> @@ -28,6 +28,8 @@
>  #include <linux/regulator/consumer.h>
>  #include <linux/slab.h>
>  
> +#include <media/rc-core.h>
> +
>  #include "sil-sii8620.h"
>  
>  #define SII8620_BURST_BUF_LEN 288
> @@ -58,6 +60,7 @@ enum sii8620_mt_state {
>  struct sii8620 {
>  	struct drm_bridge bridge;
>  	struct device *dev;
> +	struct rc_dev *rc_dev;
>  	struct clk *clk_xtal;
>  	struct gpio_desc *gpio_reset;
>  	struct gpio_desc *gpio_int;
> @@ -431,6 +434,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 +1766,25 @@ static void sii8620_send_features(struct sii8620 *ctx)
>  	sii8620_write_buf(ctx, REG_MDT_XMIT_WRITE_PORT, buf, ARRAY_SIZE(buf));
>  }
>  
> +static bool sii8620_rcp_consume(struct sii8620 *ctx, u8 scancode)
> +{
> +	bool pressed = !(scancode & MHL_RCP_KEY_RELEASED_MASK);
> +
> +	scancode &= MHL_RCP_KEY_ID_MASK;
> +
> +	if (!ctx->rc_dev) {
> +		dev_dbg(ctx->dev, "RCP input device not initialized\n");
> +		return false;
> +	}
> +
> +	if (pressed)
> +		rc_keydown(ctx->rc_dev, RC_TYPE_CEC, scancode, 0);
> +	else
> +		rc_keyup(ctx->rc_dev);
> +
> +	return true;
> +}
> +
>  static void sii8620_msc_mr_set_int(struct sii8620 *ctx)
>  {
>  	u8 ints[MHL_INT_SIZE];
> @@ -1804,19 +1836,25 @@ static void sii8620_msc_mt_done(struct sii8620 *ctx)
>  
>  static void sii8620_msc_mr_msc_msg(struct sii8620 *ctx)
>  {
> -	struct sii8620_mt_msg *msg = sii8620_msc_msg_first(ctx);
> +	struct sii8620_mt_msg *msg;
>  	u8 buf[2];
>  
> -	if (!msg)
> -		return;
> -
>  	sii8620_read_buf(ctx, REG_MSC_MR_MSC_MSG_RCVD_1ST_DATA, buf, 2);
>  
>  	switch (buf[0]) {
>  	case MHL_MSC_MSG_RAPK:
> +		msg = sii8620_msc_msg_first(ctx);
> +		if (!msg)
> +			return;
>  		msg->ret = buf[1];
>  		ctx->mt_state = MT_STATE_DONE;
>  		break;
> +	case MHL_MSC_MSG_RCP:
> +		if (!sii8620_rcp_consume(ctx, buf[1]))
> +			sii8620_mt_rcpe(ctx,
> +					MHL_RCPE_STATUS_INEFFECTIVE_KEY_CODE);
> +		sii8620_mt_rcpk(ctx, buf[1]);
> +		break;
>  	default:
>  		dev_err(ctx->dev, "%s message type %d,%d not supported",
>  			__func__, buf[0], buf[1]);
> @@ -2102,11 +2140,62 @@ static void sii8620_cable_in(struct sii8620 *ctx)
>  	enable_irq(to_i2c_client(ctx->dev)->irq);
>  }
>  
> +static void sii8620_init_rcp_input_dev(struct sii8620 *ctx)
> +{
> +	struct rc_dev *rc_dev;
> +	int ret;
> +
> +	rc_dev = rc_allocate_device(RC_DRIVER_SCANCODE);
> +	if (!rc_dev) {
> +		dev_err(ctx->dev, "Failed to allocate RC device\n");
> +		ctx->error = -ENOMEM;
> +		return;
> +	}
> +
> +	rc_dev->input_phys = "sii8620/input0";
> +	rc_dev->input_id.bustype = BUS_VIRTUAL;
> +	rc_dev->map_name = RC_MAP_CEC;
> +	rc_dev->allowed_protocols = RC_BIT_CEC;
> +	rc_dev->driver_name = "sii8620";
> +	rc_dev->input_name = "sii8620";
> +
> +	ret = rc_register_device(rc_dev);
> +	ctx->rc_dev = rc_dev;
> +
> +	if (ret) {
> +		dev_err(ctx->dev, "Failed to register RC device\n");
> +		ctx->error = ret;
> +		rc_free_device(ctx->rc_dev);
> +	}
> +}
> +
> +static void sii8620_remove_rcp_input_dev(struct sii8620 *ctx)
> +{
> +	rc_unregister_device(ctx->rc_dev);
> +	rc_free_device(ctx->rc_dev);
> +}
> +
>  static inline struct sii8620 *bridge_to_sii8620(struct drm_bridge *bridge)
>  {
>  	return container_of(bridge, struct sii8620, bridge);
>  }
>  
> +static int sii8620_attach(struct drm_bridge *bridge)
> +{
> +	struct sii8620 *ctx = bridge_to_sii8620(bridge);
> +
> +	sii8620_init_rcp_input_dev(ctx);
> +
> +	return sii8620_clear_error(ctx);
> +}
> +
> +static void sii8620_detach(struct drm_bridge *bridge)
> +{
> +	struct sii8620 *ctx = bridge_to_sii8620(bridge);
> +
> +	sii8620_remove_rcp_input_dev(ctx);
> +}
> +
>  static bool sii8620_mode_fixup(struct drm_bridge *bridge,
>  			       const struct drm_display_mode *mode,
>  			       struct drm_display_mode *adjusted_mode)
> @@ -2151,6 +2240,8 @@ static bool sii8620_mode_fixup(struct drm_bridge *bridge,
>  }
>  
>  static const struct drm_bridge_funcs sii8620_bridge_funcs = {
> +	.attach = sii8620_attach,
> +	.detach = sii8620_detach,
>  	.mode_fixup = sii8620_mode_fixup,
>  };
>  
> @@ -2217,8 +2308,8 @@ static int sii8620_remove(struct i2c_client *client)
>  	struct sii8620 *ctx = i2c_get_clientdata(client);
>  
>  	disable_irq(to_i2c_client(ctx->dev)->irq);
> -	drm_bridge_remove(&ctx->bridge);
>  	sii8620_hw_off(ctx);
> +	drm_bridge_remove(&ctx->bridge);
>  
>  	return 0;
>  }
> diff --git a/include/drm/bridge/mhl.h b/include/drm/bridge/mhl.h
> index fbdfc8d..96a5e0f 100644
> --- a/include/drm/bridge/mhl.h
> +++ b/include/drm/bridge/mhl.h
> @@ -262,6 +262,10 @@ enum {
>  #define MHL_RAPK_UNSUPPORTED	0x02	/* Rcvd RAP action code not supported */
>  #define MHL_RAPK_BUSY		0x03	/* Responder too busy to respond */
>  
> +/* Bit masks for RCP messages */
> +#define MHL_RCP_KEY_RELEASED_MASK	0x80
> +#define MHL_RCP_KEY_ID_MASK		0x7F
> +
>  /*
>   * Error status codes for RCPE messages
>   */
>
Sean Young Aug. 23, 2017, 8:39 a.m. UTC | #2
Hi,

Some review comments below.

On Wed, Aug 23, 2017 at 09:03:18AM +0200, Hans Verkuil wrote:
> Maciej,
> 
> I'm cross-posting this to linux-media and the rc maintainer Sean Young.
> 
> Note that various RC defines have been renamed in the upcoming 4.14. It might be
> better to base your code on top of https://git.linuxtv.org/media_tree.git/
> which has those patches merged.
> 
> Regards,
> 
> 	Hans
> 
> On 08/21/2017 11:33 AM, 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.
> > 
> > Changes in v2:
> > - use RC subsystem (including CEC keymap)
> > - RC device initialized in attach drm_bridge callback and
> >   removed in detach callback. This is necessary, because RC_CORE,
> >   which is needed during rc_dev init, is loaded after sii8620.
> >   DRM bridge is binded later which solves the problem.
> > - add RC_CORE dependency
> > 
> > Changes in v3:
> > - fix error handling in init_rcp and in attach callback
> > 
> > Signed-off-by: Maciej Purski <m.purski@samsung.com>
> > ---
> >  drivers/gpu/drm/bridge/Kconfig       |   2 +-
> >  drivers/gpu/drm/bridge/sil-sii8620.c | 101 +++++++++++++++++++++++++++++++++--
> >  include/drm/bridge/mhl.h             |   4 ++
> >  3 files changed, 101 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> > index adf9ae0..6ef901c 100644
> > --- a/drivers/gpu/drm/bridge/Kconfig
> > +++ b/drivers/gpu/drm/bridge/Kconfig
> > @@ -71,7 +71,7 @@ config DRM_PARADE_PS8622
> >  
> >  config DRM_SIL_SII8620
> >  	tristate "Silicon Image SII8620 HDMI/MHL bridge"
> > -	depends on OF
> > +	depends on OF && RC_CORE
> >  	select DRM_KMS_HELPER
> >  	help
> >  	  Silicon Image SII8620 HDMI/MHL bridge chip driver.
> > diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
> > index 2d51a22..e072bca 100644
> > --- a/drivers/gpu/drm/bridge/sil-sii8620.c
> > +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
> > @@ -28,6 +28,8 @@
> >  #include <linux/regulator/consumer.h>
> >  #include <linux/slab.h>
> >  
> > +#include <media/rc-core.h>
> > +
> >  #include "sil-sii8620.h"
> >  
> >  #define SII8620_BURST_BUF_LEN 288
> > @@ -58,6 +60,7 @@ enum sii8620_mt_state {
> >  struct sii8620 {
> >  	struct drm_bridge bridge;
> >  	struct device *dev;
> > +	struct rc_dev *rc_dev;
> >  	struct clk *clk_xtal;
> >  	struct gpio_desc *gpio_reset;
> >  	struct gpio_desc *gpio_int;
> > @@ -431,6 +434,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 +1766,25 @@ static void sii8620_send_features(struct sii8620 *ctx)
> >  	sii8620_write_buf(ctx, REG_MDT_XMIT_WRITE_PORT, buf, ARRAY_SIZE(buf));
> >  }
> >  
> > +static bool sii8620_rcp_consume(struct sii8620 *ctx, u8 scancode)
> > +{
> > +	bool pressed = !(scancode & MHL_RCP_KEY_RELEASED_MASK);
> > +
> > +	scancode &= MHL_RCP_KEY_ID_MASK;
> > +
> > +	if (!ctx->rc_dev) {
> > +		dev_dbg(ctx->dev, "RCP input device not initialized\n");
> > +		return false;
> > +	}
> > +
> > +	if (pressed)
> > +		rc_keydown(ctx->rc_dev, RC_TYPE_CEC, scancode, 0);
> > +	else
> > +		rc_keyup(ctx->rc_dev);
> > +
> > +	return true;
> > +}
> > +
> >  static void sii8620_msc_mr_set_int(struct sii8620 *ctx)
> >  {
> >  	u8 ints[MHL_INT_SIZE];
> > @@ -1804,19 +1836,25 @@ static void sii8620_msc_mt_done(struct sii8620 *ctx)
> >  
> >  static void sii8620_msc_mr_msc_msg(struct sii8620 *ctx)
> >  {
> > -	struct sii8620_mt_msg *msg = sii8620_msc_msg_first(ctx);
> > +	struct sii8620_mt_msg *msg;
> >  	u8 buf[2];
> >  
> > -	if (!msg)
> > -		return;
> > -
> >  	sii8620_read_buf(ctx, REG_MSC_MR_MSC_MSG_RCVD_1ST_DATA, buf, 2);
> >  
> >  	switch (buf[0]) {
> >  	case MHL_MSC_MSG_RAPK:
> > +		msg = sii8620_msc_msg_first(ctx);
> > +		if (!msg)
> > +			return;
> >  		msg->ret = buf[1];
> >  		ctx->mt_state = MT_STATE_DONE;
> >  		break;
> > +	case MHL_MSC_MSG_RCP:
> > +		if (!sii8620_rcp_consume(ctx, buf[1]))
> > +			sii8620_mt_rcpe(ctx,
> > +					MHL_RCPE_STATUS_INEFFECTIVE_KEY_CODE);
> > +		sii8620_mt_rcpk(ctx, buf[1]);
> > +		break;
> >  	default:
> >  		dev_err(ctx->dev, "%s message type %d,%d not supported",
> >  			__func__, buf[0], buf[1]);
> > @@ -2102,11 +2140,62 @@ static void sii8620_cable_in(struct sii8620 *ctx)
> >  	enable_irq(to_i2c_client(ctx->dev)->irq);
> >  }
> >  
> > +static void sii8620_init_rcp_input_dev(struct sii8620 *ctx)
> > +{
> > +	struct rc_dev *rc_dev;
> > +	int ret;
> > +
> > +	rc_dev = rc_allocate_device(RC_DRIVER_SCANCODE);
> > +	if (!rc_dev) {
> > +		dev_err(ctx->dev, "Failed to allocate RC device\n");
> > +		ctx->error = -ENOMEM;
> > +		return;
> > +	}
> > +
> > +	rc_dev->input_phys = "sii8620/input0";
> > +	rc_dev->input_id.bustype = BUS_VIRTUAL;
> > +	rc_dev->map_name = RC_MAP_CEC;
> > +	rc_dev->allowed_protocols = RC_BIT_CEC;
> > +	rc_dev->driver_name = "sii8620";
> > +	rc_dev->input_name = "sii8620";

input_name is already renamed to device_name (just like RC_BIT_CEC is
RC_PROTO_BIT_CEC now, as Hans pointed out already).

> > +
> > +	ret = rc_register_device(rc_dev);
> > +	ctx->rc_dev = rc_dev;
> > +
> > +	if (ret) {
> > +		dev_err(ctx->dev, "Failed to register RC device\n");
> > +		ctx->error = ret;
> > +		rc_free_device(ctx->rc_dev);
> > +	}

In this error path, ctx->rc_dev is still set but contains a pointer to
something which is already freed.

> > +}
> > +
> > +static void sii8620_remove_rcp_input_dev(struct sii8620 *ctx)
> > +{
> > +	rc_unregister_device(ctx->rc_dev);
> > +	rc_free_device(ctx->rc_dev);

rc_unregister_device() already frees the rc_dev, don't call rc_free_device()
as well.

> > +}
> > +
> >  static inline struct sii8620 *bridge_to_sii8620(struct drm_bridge *bridge)
> >  {
> >  	return container_of(bridge, struct sii8620, bridge);
> >  }
> >  
> > +static int sii8620_attach(struct drm_bridge *bridge)
> > +{
> > +	struct sii8620 *ctx = bridge_to_sii8620(bridge);
> > +
> > +	sii8620_init_rcp_input_dev(ctx);
> > +
> > +	return sii8620_clear_error(ctx);
> > +}
> > +
> > +static void sii8620_detach(struct drm_bridge *bridge)
> > +{
> > +	struct sii8620 *ctx = bridge_to_sii8620(bridge);
> > +
> > +	sii8620_remove_rcp_input_dev(ctx);
> > +}
> > +
> >  static bool sii8620_mode_fixup(struct drm_bridge *bridge,
> >  			       const struct drm_display_mode *mode,
> >  			       struct drm_display_mode *adjusted_mode)
> > @@ -2151,6 +2240,8 @@ static bool sii8620_mode_fixup(struct drm_bridge *bridge,
> >  }
> >  
> >  static const struct drm_bridge_funcs sii8620_bridge_funcs = {
> > +	.attach = sii8620_attach,
> > +	.detach = sii8620_detach,
> >  	.mode_fixup = sii8620_mode_fixup,
> >  };
> >  
> > @@ -2217,8 +2308,8 @@ static int sii8620_remove(struct i2c_client *client)
> >  	struct sii8620 *ctx = i2c_get_clientdata(client);
> >  
> >  	disable_irq(to_i2c_client(ctx->dev)->irq);
> > -	drm_bridge_remove(&ctx->bridge);
> >  	sii8620_hw_off(ctx);
> > +	drm_bridge_remove(&ctx->bridge);
> >  
> >  	return 0;
> >  }
> > diff --git a/include/drm/bridge/mhl.h b/include/drm/bridge/mhl.h
> > index fbdfc8d..96a5e0f 100644
> > --- a/include/drm/bridge/mhl.h
> > +++ b/include/drm/bridge/mhl.h
> > @@ -262,6 +262,10 @@ enum {
> >  #define MHL_RAPK_UNSUPPORTED	0x02	/* Rcvd RAP action code not supported */
> >  #define MHL_RAPK_BUSY		0x03	/* Responder too busy to respond */
> >  
> > +/* Bit masks for RCP messages */
> > +#define MHL_RCP_KEY_RELEASED_MASK	0x80
> > +#define MHL_RCP_KEY_ID_MASK		0x7F
> > +
> >  /*
> >   * Error status codes for RCPE messages
> >   */
> >
diff mbox

Patch

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index adf9ae0..6ef901c 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -71,7 +71,7 @@  config DRM_PARADE_PS8622
 
 config DRM_SIL_SII8620
 	tristate "Silicon Image SII8620 HDMI/MHL bridge"
-	depends on OF
+	depends on OF && RC_CORE
 	select DRM_KMS_HELPER
 	help
 	  Silicon Image SII8620 HDMI/MHL bridge chip driver.
diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
index 2d51a22..e072bca 100644
--- a/drivers/gpu/drm/bridge/sil-sii8620.c
+++ b/drivers/gpu/drm/bridge/sil-sii8620.c
@@ -28,6 +28,8 @@ 
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 
+#include <media/rc-core.h>
+
 #include "sil-sii8620.h"
 
 #define SII8620_BURST_BUF_LEN 288
@@ -58,6 +60,7 @@  enum sii8620_mt_state {
 struct sii8620 {
 	struct drm_bridge bridge;
 	struct device *dev;
+	struct rc_dev *rc_dev;
 	struct clk *clk_xtal;
 	struct gpio_desc *gpio_reset;
 	struct gpio_desc *gpio_int;
@@ -431,6 +434,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 +1766,25 @@  static void sii8620_send_features(struct sii8620 *ctx)
 	sii8620_write_buf(ctx, REG_MDT_XMIT_WRITE_PORT, buf, ARRAY_SIZE(buf));
 }
 
+static bool sii8620_rcp_consume(struct sii8620 *ctx, u8 scancode)
+{
+	bool pressed = !(scancode & MHL_RCP_KEY_RELEASED_MASK);
+
+	scancode &= MHL_RCP_KEY_ID_MASK;
+
+	if (!ctx->rc_dev) {
+		dev_dbg(ctx->dev, "RCP input device not initialized\n");
+		return false;
+	}
+
+	if (pressed)
+		rc_keydown(ctx->rc_dev, RC_TYPE_CEC, scancode, 0);
+	else
+		rc_keyup(ctx->rc_dev);
+
+	return true;
+}
+
 static void sii8620_msc_mr_set_int(struct sii8620 *ctx)
 {
 	u8 ints[MHL_INT_SIZE];
@@ -1804,19 +1836,25 @@  static void sii8620_msc_mt_done(struct sii8620 *ctx)
 
 static void sii8620_msc_mr_msc_msg(struct sii8620 *ctx)
 {
-	struct sii8620_mt_msg *msg = sii8620_msc_msg_first(ctx);
+	struct sii8620_mt_msg *msg;
 	u8 buf[2];
 
-	if (!msg)
-		return;
-
 	sii8620_read_buf(ctx, REG_MSC_MR_MSC_MSG_RCVD_1ST_DATA, buf, 2);
 
 	switch (buf[0]) {
 	case MHL_MSC_MSG_RAPK:
+		msg = sii8620_msc_msg_first(ctx);
+		if (!msg)
+			return;
 		msg->ret = buf[1];
 		ctx->mt_state = MT_STATE_DONE;
 		break;
+	case MHL_MSC_MSG_RCP:
+		if (!sii8620_rcp_consume(ctx, buf[1]))
+			sii8620_mt_rcpe(ctx,
+					MHL_RCPE_STATUS_INEFFECTIVE_KEY_CODE);
+		sii8620_mt_rcpk(ctx, buf[1]);
+		break;
 	default:
 		dev_err(ctx->dev, "%s message type %d,%d not supported",
 			__func__, buf[0], buf[1]);
@@ -2102,11 +2140,62 @@  static void sii8620_cable_in(struct sii8620 *ctx)
 	enable_irq(to_i2c_client(ctx->dev)->irq);
 }
 
+static void sii8620_init_rcp_input_dev(struct sii8620 *ctx)
+{
+	struct rc_dev *rc_dev;
+	int ret;
+
+	rc_dev = rc_allocate_device(RC_DRIVER_SCANCODE);
+	if (!rc_dev) {
+		dev_err(ctx->dev, "Failed to allocate RC device\n");
+		ctx->error = -ENOMEM;
+		return;
+	}
+
+	rc_dev->input_phys = "sii8620/input0";
+	rc_dev->input_id.bustype = BUS_VIRTUAL;
+	rc_dev->map_name = RC_MAP_CEC;
+	rc_dev->allowed_protocols = RC_BIT_CEC;
+	rc_dev->driver_name = "sii8620";
+	rc_dev->input_name = "sii8620";
+
+	ret = rc_register_device(rc_dev);
+	ctx->rc_dev = rc_dev;
+
+	if (ret) {
+		dev_err(ctx->dev, "Failed to register RC device\n");
+		ctx->error = ret;
+		rc_free_device(ctx->rc_dev);
+	}
+}
+
+static void sii8620_remove_rcp_input_dev(struct sii8620 *ctx)
+{
+	rc_unregister_device(ctx->rc_dev);
+	rc_free_device(ctx->rc_dev);
+}
+
 static inline struct sii8620 *bridge_to_sii8620(struct drm_bridge *bridge)
 {
 	return container_of(bridge, struct sii8620, bridge);
 }
 
+static int sii8620_attach(struct drm_bridge *bridge)
+{
+	struct sii8620 *ctx = bridge_to_sii8620(bridge);
+
+	sii8620_init_rcp_input_dev(ctx);
+
+	return sii8620_clear_error(ctx);
+}
+
+static void sii8620_detach(struct drm_bridge *bridge)
+{
+	struct sii8620 *ctx = bridge_to_sii8620(bridge);
+
+	sii8620_remove_rcp_input_dev(ctx);
+}
+
 static bool sii8620_mode_fixup(struct drm_bridge *bridge,
 			       const struct drm_display_mode *mode,
 			       struct drm_display_mode *adjusted_mode)
@@ -2151,6 +2240,8 @@  static bool sii8620_mode_fixup(struct drm_bridge *bridge,
 }
 
 static const struct drm_bridge_funcs sii8620_bridge_funcs = {
+	.attach = sii8620_attach,
+	.detach = sii8620_detach,
 	.mode_fixup = sii8620_mode_fixup,
 };
 
@@ -2217,8 +2308,8 @@  static int sii8620_remove(struct i2c_client *client)
 	struct sii8620 *ctx = i2c_get_clientdata(client);
 
 	disable_irq(to_i2c_client(ctx->dev)->irq);
-	drm_bridge_remove(&ctx->bridge);
 	sii8620_hw_off(ctx);
+	drm_bridge_remove(&ctx->bridge);
 
 	return 0;
 }
diff --git a/include/drm/bridge/mhl.h b/include/drm/bridge/mhl.h
index fbdfc8d..96a5e0f 100644
--- a/include/drm/bridge/mhl.h
+++ b/include/drm/bridge/mhl.h
@@ -262,6 +262,10 @@  enum {
 #define MHL_RAPK_UNSUPPORTED	0x02	/* Rcvd RAP action code not supported */
 #define MHL_RAPK_BUSY		0x03	/* Responder too busy to respond */
 
+/* Bit masks for RCP messages */
+#define MHL_RCP_KEY_RELEASED_MASK	0x80
+#define MHL_RCP_KEY_ID_MASK		0x7F
+
 /*
  * Error status codes for RCPE messages
  */