Re: [PATCH 2/4] drm/msm: add hdmi cec support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Le jeu. 20 avr. 2023 à 02:20, Dmitry Baryshkov
<dmitry.baryshkov@xxxxxxxxxx> a écrit :
>
> On 18/04/2023 21:10, Arnaud Vrac wrote:
> > Some Qualcomm SoCs that support HDMI also support CEC, including MSM8996
> > and MSM8998. The hardware block can handle a single CEC logical address
> > and broadcast messages.
> >
> > Port the CEC driver from downstream msm-4.4 kernel. It has been tested
> > on MSM8998 and passes the cec-compliance tool tests.
> >
> > Signed-off-by: Arnaud Vrac <avrac@xxxxxxxxxx>
> > ---
> >   drivers/gpu/drm/msm/Kconfig         |   8 ++
> >   drivers/gpu/drm/msm/Makefile        |   1 +
> >   drivers/gpu/drm/msm/hdmi/hdmi.c     |  15 ++
> >   drivers/gpu/drm/msm/hdmi/hdmi.h     |  18 +++
> >   drivers/gpu/drm/msm/hdmi/hdmi_cec.c | 280 ++++++++++++++++++++++++++++++++++++
> >   5 files changed, 322 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
> > index 85f5ab1d552c4..2a02c74207935 100644
> > --- a/drivers/gpu/drm/msm/Kconfig
> > +++ b/drivers/gpu/drm/msm/Kconfig
> > @@ -165,3 +165,11 @@ config DRM_MSM_HDMI_HDCP
> >       default y
> >       help
> >         Choose this option to enable HDCP state machine
> > +
> > +config DRM_MSM_HDMI_CEC
> > +     bool "Enable HDMI CEC support in MSM DRM driver"
> > +     depends on DRM_MSM && DRM_MSM_HDMI
> > +     select CEC_CORE
> > +     default y
> > +     help
> > +       Choose this option to enable CEC support
> > diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
> > index 7274c41228ed9..0237a2f219ac2 100644
> > --- a/drivers/gpu/drm/msm/Makefile
> > +++ b/drivers/gpu/drm/msm/Makefile
> > @@ -131,6 +131,7 @@ msm-$(CONFIG_DRM_MSM_DP)+= dp/dp_aux.o \
> >
> >   msm-$(CONFIG_DRM_FBDEV_EMULATION) += msm_fbdev.o
> >
> > +msm-$(CONFIG_DRM_MSM_HDMI_CEC) += hdmi/hdmi_cec.o
> >   msm-$(CONFIG_DRM_MSM_HDMI_HDCP) += hdmi/hdmi_hdcp.o
> >
> >   msm-$(CONFIG_DRM_MSM_DSI) += dsi/dsi.o \
> > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
> > index 3132105a2a433..1dde3890e25c0 100644
> > --- a/drivers/gpu/drm/msm/hdmi/hdmi.c
> > +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
> > @@ -11,6 +11,8 @@
> >   #include <drm/drm_bridge_connector.h>
> >   #include <drm/drm_of.h>
> >
> > +#include <media/cec.h>
> > +
> >   #include <sound/hdmi-codec.h>
> >   #include "hdmi.h"
> >
> > @@ -53,6 +55,9 @@ static irqreturn_t msm_hdmi_irq(int irq, void *dev_id)
> >       if (hdmi->hdcp_ctrl)
> >               msm_hdmi_hdcp_irq(hdmi->hdcp_ctrl);
> >
> > +     /* Process CEC: */
> > +     msm_hdmi_cec_irq(hdmi);
> > +
> >       /* TODO audio.. */
> >
> >       return IRQ_HANDLED;
> > @@ -66,6 +71,8 @@ static void msm_hdmi_destroy(struct hdmi *hdmi)
> >        */
> >       if (hdmi->workq)
> >               destroy_workqueue(hdmi->workq);
> > +
> > +     msm_hdmi_cec_exit(hdmi);
> >       msm_hdmi_hdcp_destroy(hdmi);
> >
> >       if (hdmi->i2c)
> > @@ -139,6 +146,8 @@ static int msm_hdmi_init(struct hdmi *hdmi)
> >               hdmi->hdcp_ctrl = NULL;
> >       }
> >
> > +     msm_hdmi_cec_init(hdmi);
> > +
> >       return 0;
> >
> >   fail:
> > @@ -198,6 +207,12 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
> >
> >       drm_connector_attach_encoder(hdmi->connector, hdmi->encoder);
> >
> > +     if (hdmi->cec_adap) {
> > +             struct cec_connector_info conn_info;
> > +             cec_fill_conn_info_from_drm(&conn_info, hdmi->connector);
> > +             cec_s_conn_info(hdmi->cec_adap, &conn_info);
> > +     }
> > +
> >       ret = devm_request_irq(dev->dev, hdmi->irq,
> >                       msm_hdmi_irq, IRQF_TRIGGER_HIGH,
> >                       "hdmi_isr", hdmi);
> > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
> > index e8dbee50637fa..c639bd87f4b8f 100644
> > --- a/drivers/gpu/drm/msm/hdmi/hdmi.h
> > +++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
> > @@ -29,6 +29,7 @@ struct hdmi_audio {
> >   };
> >
> >   struct hdmi_hdcp_ctrl;
> > +struct cec_adapter;
> >
> >   struct hdmi {
> >       struct drm_device *dev;
> > @@ -73,6 +74,7 @@ struct hdmi {
> >       struct workqueue_struct *workq;
> >
> >       struct hdmi_hdcp_ctrl *hdcp_ctrl;
> > +     struct cec_adapter *cec_adap;
> >
> >       /*
> >       * spinlock to protect registers shared by different execution
> > @@ -261,4 +263,20 @@ static inline void msm_hdmi_hdcp_off(struct hdmi_hdcp_ctrl *hdcp_ctrl) {}
> >   static inline void msm_hdmi_hdcp_irq(struct hdmi_hdcp_ctrl *hdcp_ctrl) {}
> >   #endif
> >
> > +/*
> > + * cec
> > + */
> > +#ifdef CONFIG_DRM_MSM_HDMI_CEC
> > +int msm_hdmi_cec_init(struct hdmi *hdmi);
> > +void msm_hdmi_cec_exit(struct hdmi *hdmi);
> > +void msm_hdmi_cec_irq(struct hdmi *hdmi);
> > +#else
> > +static inline int msm_hdmi_cec_init(struct hdmi *hdmi)
> > +{
> > +     return -ENXIO;
> > +}
> > +static inline void msm_hdmi_cec_exit(struct hdmi *hdmi) {}
> > +static inline void msm_hdmi_cec_irq(struct hdmi *hdmi) {}
> > +#endif
> > +
> >   #endif /* __HDMI_CONNECTOR_H__ */
> > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_cec.c b/drivers/gpu/drm/msm/hdmi/hdmi_cec.c
> > new file mode 100644
> > index 0000000000000..51326e493e5da
> > --- /dev/null
> > +++ b/drivers/gpu/drm/msm/hdmi/hdmi_cec.c
> > @@ -0,0 +1,280 @@
> > +#include <linux/iopoll.h>
> > +#include <media/cec.h>
> > +
> > +#include "hdmi.h"
> > +
> > +#define HDMI_CEC_INT_MASK ( \
> > +     HDMI_CEC_INT_TX_DONE_MASK | \
> > +     HDMI_CEC_INT_TX_ERROR_MASK | \
> > +     HDMI_CEC_INT_RX_DONE_MASK)
> > +
> > +struct hdmi_cec_ctrl {
> > +     struct hdmi *hdmi;
> > +     struct work_struct work;
> > +     spinlock_t lock;
> > +     u32 irq_status;
> > +     u32 tx_status;
> > +     u32 tx_retransmits;
> > +};
> > +
> > +static int msm_hdmi_cec_adap_enable(struct cec_adapter *adap, bool enable)
> > +{
> > +     struct hdmi_cec_ctrl *cec_ctrl = adap->priv;
> > +     struct hdmi *hdmi = cec_ctrl->hdmi;
> > +
> > +     if (enable) {
> > +             /* timer frequency, 19.2Mhz * 0.05ms / 1000ms = 960 */
> > +             hdmi_write(hdmi, REG_HDMI_CEC_REFTIMER,
> > +                        HDMI_CEC_REFTIMER_REFTIMER(960) |
> > +                        HDMI_CEC_REFTIMER_ENABLE);
> > +
> > +             /* read and write timings */
> > +             hdmi_write(hdmi, REG_HDMI_CEC_RD_RANGE, 0x30AB9888);
>
> lowercase hex please. We are trying to switch to it.
>
> > +             hdmi_write(hdmi, REG_HDMI_CEC_WR_RANGE, 0x888AA888);
> > +             hdmi_write(hdmi, REG_HDMI_CEC_RD_START_RANGE, 0x88888888);
> > +             hdmi_write(hdmi, REG_HDMI_CEC_RD_TOTAL_RANGE, 0x99);
> > +
> > +             /* start bit low pulse duration, 3.7ms */
> > +             hdmi_write(hdmi, REG_HDMI_CEC_RD_ERR_RESP_LO, 74);
> > +
> > +             /* signal free time, 7 * 2.4ms */
> > +             hdmi_write(hdmi, REG_HDMI_CEC_TIME,
> > +                        HDMI_CEC_TIME_SIGNAL_FREE_TIME(7 * 48) |
> > +                        HDMI_CEC_TIME_ENABLE);
> > +
> > +             hdmi_write(hdmi, REG_HDMI_CEC_COMPL_CTL, 0xF);
> > +             hdmi_write(hdmi, REG_HDMI_CEC_WR_CHECK_CONFIG, 0x4);
> > +             hdmi_write(hdmi, REG_HDMI_CEC_RD_FILTER, BIT(0) | (0x7FF << 4));
> > +
> > +             hdmi_write(hdmi, REG_HDMI_CEC_INT, HDMI_CEC_INT_MASK);
> > +             hdmi_write(hdmi, REG_HDMI_CEC_CTRL, HDMI_CEC_CTRL_ENABLE);
> > +     } else {
> > +             hdmi_write(hdmi, REG_HDMI_CEC_INT, 0);
> > +             hdmi_write(hdmi, REG_HDMI_CEC_CTRL, 0);
> > +             cancel_work_sync(&cec_ctrl->work);
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static int msm_hdmi_cec_adap_log_addr(struct cec_adapter *adap, u8 logical_addr)
> > +{
> > +     struct hdmi_cec_ctrl *cec_ctrl = adap->priv;
> > +     struct hdmi *hdmi = cec_ctrl->hdmi;
> > +
> > +     hdmi_write(hdmi, REG_HDMI_CEC_ADDR, logical_addr & 0xF);
> > +
> > +     return 0;
> > +}
> > +
> > +static int msm_hdmi_cec_adap_transmit(struct cec_adapter *adap, u8 attempts,
> > +                                   u32 signal_free_time, struct cec_msg *msg)
> > +{
> > +     struct hdmi_cec_ctrl *cec_ctrl = adap->priv;
> > +     struct hdmi *hdmi = cec_ctrl->hdmi;
> > +     u8 retransmits;
> > +     u32 broadcast;
> > +     u32 status;
> > +     int i;
> > +
> > +     /* toggle cec in order to flush out bad hw state, if any */
> > +     hdmi_write(hdmi, REG_HDMI_CEC_CTRL, 0);
> > +     hdmi_write(hdmi, REG_HDMI_CEC_CTRL, HDMI_CEC_CTRL_ENABLE);
> > +
> > +     /* flush register writes */
> > +     wmb();
> > +
> > +     retransmits = attempts ? (attempts - 1) : 0;
> > +     hdmi_write(hdmi, REG_HDMI_CEC_RETRANSMIT,
> > +                HDMI_CEC_RETRANSMIT_ENABLE |
> > +                HDMI_CEC_RETRANSMIT_COUNT(retransmits));
> > +
> > +     broadcast = cec_msg_is_broadcast(msg) ? HDMI_CEC_WR_DATA_BROADCAST : 0;
> > +     for (i = 0; i < msg->len; i++) {
> > +             hdmi_write(hdmi, REG_HDMI_CEC_WR_DATA,
> > +                        HDMI_CEC_WR_DATA_DATA(msg->msg[i]) | broadcast);
> > +     }
> > +
> > +     /* check line status */
> > +     if (read_poll_timeout(hdmi_read, status, !(status & HDMI_CEC_STATUS_BUSY),
> > +                           5, 1000, false, hdmi, REG_HDMI_CEC_STATUS)) {
> > +             pr_err("CEC line is busy. Retry failed\n");
> > +             return -EBUSY;
> > +     }
> > +
> > +     cec_ctrl->tx_retransmits = retransmits;
> > +
> > +     /* start transmission */
> > +     hdmi_write(hdmi, REG_HDMI_CEC_CTRL,
> > +                HDMI_CEC_CTRL_ENABLE |
> > +                HDMI_CEC_CTRL_SEND_TRIGGER |
> > +                HDMI_CEC_CTRL_FRAME_SIZE(msg->len) |
> > +                HDMI_CEC_CTRL_LINE_OE);
> > +
> > +     return 0;
> > +}
> > +
> > +static void msm_hdmi_cec_adap_free(struct cec_adapter *adap)
> > +{
> > +     struct hdmi_cec_ctrl *cec_ctrl = adap->priv;
> > +
> > +     cec_ctrl->hdmi->cec_adap = NULL;
> > +     kfree(cec_ctrl);
> > +}
> > +
> > +static const struct cec_adap_ops msm_hdmi_cec_adap_ops = {
> > +     .adap_enable = msm_hdmi_cec_adap_enable,
> > +     .adap_log_addr = msm_hdmi_cec_adap_log_addr,
> > +     .adap_transmit = msm_hdmi_cec_adap_transmit,
> > +     .adap_free = msm_hdmi_cec_adap_free,
> > +};
> > +
> > +#define CEC_IRQ_FRAME_WR_DONE 0x01
> > +#define CEC_IRQ_FRAME_RD_DONE 0x02
>
> Please move these to top. Also you might consider replacing this mask
> with two boolean flags.
>
> > +
> > +static void msm_hdmi_cec_handle_rx_done(struct hdmi_cec_ctrl *cec_ctrl)
> > +{
> > +     struct hdmi *hdmi = cec_ctrl->hdmi;
> > +     struct cec_msg msg = {};
> > +     u32 data;
> > +     int i;
> > +
> > +     data = hdmi_read(hdmi, REG_HDMI_CEC_RD_DATA);
> > +     msg.len = (data & 0x1f00) >> 8;
>
> We can also use FIELD_GET here. I'll add defines to the mesa merge request.

Ok, I wasn't sure if it made sense to add the bitfield definition for
this since it's only valid for the first byte of the message. I'll add
the change since this has been integrated into mesa.

>
> > +     if (msg.len < 1 || msg.len > CEC_MAX_MSG_SIZE)
> > +             return;
> > +
> > +     msg.msg[0] = data & 0xff;
> > +     for (i = 1; i < msg.len; i++)
> > +             msg.msg[i] = hdmi_read(hdmi, REG_HDMI_CEC_RD_DATA) & 0xff;
> > +
> > +     cec_received_msg(hdmi->cec_adap, &msg);
> > +}
> > +
> > +static void msm_hdmi_cec_handle_tx_done(struct hdmi_cec_ctrl *cec_ctrl)
> > +{
> > +     struct hdmi *hdmi = cec_ctrl->hdmi;
> > +     u32 tx_status;
> > +
> > +     tx_status = (cec_ctrl->tx_status & HDMI_CEC_STATUS_TX_STATUS__MASK) >>
> > +             HDMI_CEC_STATUS_TX_STATUS__SHIFT;
>
> FIELD_GET(HDMI_CEC_STATUS_TX_STATUS__MASK, cec_ctrl->tx_status).
>
> > +
> > +     switch (tx_status) {
> > +     case 0:
>
> Please use valus from enum hdmi_cec_tx_status
>
> > +             cec_transmit_done(hdmi->cec_adap,
> > +                               CEC_TX_STATUS_OK, 0, 0, 0, 0);
> > +             break;
> > +     case 1:
> > +             cec_transmit_done(hdmi->cec_adap,
> > +                               CEC_TX_STATUS_NACK, 0, 1, 0, 0);
> > +             break;
> > +     case 2:
> > +             cec_transmit_done(hdmi->cec_adap,
> > +                               CEC_TX_STATUS_ARB_LOST, 1, 0, 0, 0);
> > +             break;
> > +     case 3:
> > +             cec_transmit_done(hdmi->cec_adap,
> > +                               CEC_TX_STATUS_MAX_RETRIES |
> > +                               CEC_TX_STATUS_NACK,
> > +                               0, cec_ctrl->tx_retransmits + 1, 0, 0);
> > +             break;
> > +     default:
> > +             cec_transmit_done(hdmi->cec_adap,
> > +                               CEC_TX_STATUS_ERROR, 0, 0, 0, 1);
> > +             break;
> > +     }
> > +}
> > +
> > +static void msm_hdmi_cec_work(struct work_struct *work)
> > +{
> > +     struct hdmi_cec_ctrl *cec_ctrl =
> > +             container_of(work, struct hdmi_cec_ctrl, work);
> > +     unsigned long flags;
> > +
> > +     spin_lock_irqsave(&cec_ctrl->lock, flags);
> > +
> > +     if (cec_ctrl->irq_status & CEC_IRQ_FRAME_WR_DONE)
> > +             msm_hdmi_cec_handle_tx_done(cec_ctrl);
> > +
> > +     if (cec_ctrl->irq_status & CEC_IRQ_FRAME_RD_DONE)
> > +             msm_hdmi_cec_handle_rx_done(cec_ctrl);
> > +
> > +     cec_ctrl->irq_status = 0;
> > +     cec_ctrl->tx_status = 0;
> > +
> > +     spin_unlock_irqrestore(&cec_ctrl->lock, flags);
> > +}
> > +
> > +void msm_hdmi_cec_irq(struct hdmi *hdmi)
> > +{
> > +     struct hdmi_cec_ctrl *cec_ctrl;
> > +     unsigned long flags;
> > +     u32 int_status;
> > +
> > +     if (!hdmi->cec_adap)
> > +             return;
> > +
> > +     cec_ctrl = hdmi->cec_adap->priv;
> > +
> > +     int_status = hdmi_read(hdmi, REG_HDMI_CEC_INT);
> > +     if (!(int_status & HDMI_CEC_INT_MASK))
> > +             return;
> > +
> > +     spin_lock_irqsave(&cec_ctrl->lock, flags);
> > +
> > +     if (int_status & (HDMI_CEC_INT_TX_DONE | HDMI_CEC_INT_TX_ERROR)) {
> > +             cec_ctrl->tx_status = hdmi_read(hdmi, REG_HDMI_CEC_STATUS);
> > +             cec_ctrl->irq_status |= CEC_IRQ_FRAME_WR_DONE;
> > +     }
> > +
> > +     if (int_status & HDMI_CEC_INT_RX_DONE)
> > +             cec_ctrl->irq_status |= CEC_IRQ_FRAME_RD_DONE;
> > +
> > +     spin_unlock_irqrestore(&cec_ctrl->lock, flags);
> > +
> > +     hdmi_write(hdmi, REG_HDMI_CEC_INT, int_status);
> > +     queue_work(hdmi->workq, &cec_ctrl->work);
> > +}
> > +
> > +int msm_hdmi_cec_init(struct hdmi *hdmi)
> > +{
> > +     struct platform_device *pdev = hdmi->pdev;
> > +     struct hdmi_cec_ctrl *cec_ctrl;
> > +     struct cec_adapter *cec_adap;
> > +     int ret;
>
> hdmi_cec_enable from msm-4.4 tells that CEC is not supported if
> REG_HDMI_VERSION reads < 0x30000001. Could you please add this check?

Ack. I wonder which SoCs before MSM8996 can actually support CEC.

>
>
>
>
> > +
> > +     cec_ctrl = kzalloc(sizeof (*cec_ctrl), GFP_KERNEL);
> > +     if (!cec_ctrl)
> > +             return -ENOMEM;
> > +
> > +     cec_ctrl->hdmi = hdmi;
> > +     INIT_WORK(&cec_ctrl->work, msm_hdmi_cec_work);
> > +
> > +     cec_adap = cec_allocate_adapter(&msm_hdmi_cec_adap_ops,
> > +                                     cec_ctrl, "msm",
> > +                                     CEC_CAP_DEFAULTS |
> > +                                     CEC_CAP_CONNECTOR_INFO, 1);
> > +     ret = PTR_ERR_OR_ZERO(cec_adap);
> > +     if (ret < 0) {
> > +             kfree(cec_ctrl);
> > +             return ret;
> > +     }
> > +
> > +     /* Set the logical address to Unregistered */
> > +     hdmi_write(hdmi, REG_HDMI_CEC_ADDR, 0xf);
> > +
> > +     ret = cec_register_adapter(cec_adap, &pdev->dev);
> > +     if (ret < 0) {
> > +             cec_delete_adapter(cec_adap);
> > +             return ret;
> > +     }
> > +
> > +     hdmi->cec_adap = cec_adap;
> > +
> > +     return 0;
> > +}
> > +
> > +void msm_hdmi_cec_exit(struct hdmi *hdmi)
> > +{
> > +     cec_unregister_adapter(hdmi->cec_adap);
> > +}
> >
>
> --
> With best wishes
> Dmitry
>




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux