On Thu, May 02, 2019 at 03:53:33PM -0700, Douglas Anderson wrote:
> See the PhD thesis in the comments in this patch for details, but to
> summarize this adds a hacky "unwedge" feature to the dw_hdmi i2c bus to
> workaround what appears to be a hardware errata. This relies on a
> pinctrl entry to help change around muxing to perform the unwedge.
>
> NOTE that the specific TV this was tested on was the "Samsung
> UN40HU6950FXZA" and the specific port was the "STB" port.
>
> Signed-off-by: Douglas Anderson <diand...@chromium.org>
> ---
>
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 116 +++++++++++++++++++---
> 1 file changed, 100 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index db761329a1e3..c66587e33813 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -19,6 +19,7 @@
> #include <linux/hdmi.h>
> #include <linux/mutex.h>
> #include <linux/of_device.h>
> +#include <linux/pinctrl/consumer.h>
> #include <linux/regmap.h>
> #include <linux/spinlock.h>
>
> @@ -169,6 +170,10 @@ struct dw_hdmi {
> bool sink_is_hdmi;
> bool sink_has_audio;
>
> + struct pinctrl *pinctrl;
> + struct pinctrl_state *default_state;
> + struct pinctrl_state *unwedge_state;
> +
> struct mutex mutex; /* for state below and previous_mode */
> enum drm_connector_force force; /* mutex-protected force state */
> bool disabled; /* DRM has disabled our bridge */
> @@ -247,11 +252,82 @@ static void dw_hdmi_i2c_init(struct dw_hdmi *hdmi)
> HDMI_IH_MUTE_I2CM_STAT0);
> }
>
> +static bool dw_hdmi_i2c_unwedge(struct dw_hdmi *hdmi)
> +{
> + /* If no unwedge state then give up */
> + if (IS_ERR(hdmi->unwedge_state))
> + return false;
> +
> + dev_info(hdmi->dev, "Attempting to unwedge stuck i2c bus\n");
> +
> + /*
> + * This is a huge hack to workaround a problem where the dw_hdmi i2c
> + * bus could sometimes get wedged. Once wedged there doesn't appear
> + * to be any way to unwedge it (including the HDMI_I2CM_SOFTRSTZ)
> + * other than pulsing the SDA line.
> + *
> + * We appear to be able to pulse the SDA line (in the eyes of dw_hdmi)
> + * by:
> + * 1. Remux the pin as a GPIO output, driven low.
> + * 2. Wait a little while. 1 ms seems to work, but we'll do 10.
> + * 3. Immediately jump to remux the pin as dw_hdmi i2c again.
> + *
> + * At the moment of remuxing, the line will still be low due to its
> + * recent stint as an output, but then it will be pulled high by the
> + * (presumed) external pullup. dw_hdmi seems to see this as a rising
> + * edge and that seems to get it out of its jam.
> + *
> + * This wedging was only ever seen on one TV, and only on one of
> + * its HDMI ports. It happened when the TV was powered on while the
> + * device was plugged in. A scope trace shows the TV bringing both SDA
> + * and SCL low, then bringing them both back up at roughly the same
> + * time. Presumably this confuses dw_hdmi because it saw activity but
> + * no real STOP (maybe it thinks there's another master on the bus?).
> + * Giving it a clean rising edge of SDA while SCL is already high
> + * presumably makes dw_hdmi see a STOP which seems to bring dw_hdmi out
> + * of its stupor.
> + *
> + * Note that after coming back alive, transfers seem to immediately
> + * resume, so if we unwedge due to a timeout we should wait a little
> + * longer for our transfer to finish, since it might have just started
> + * now.
> + */
> + pinctrl_select_state(hdmi->pinctrl, hdmi->unwedge_state);
> + msleep(10);
> + pinctrl_select_state(hdmi->pinctrl, hdmi->default_state);
> +
> + return true;
> +}
> +
> +static int dw_hdmi_i2c_wait(struct dw_hdmi *hdmi)
> +{
> + struct dw_hdmi_i2c *i2c = hdmi->i2c;
> + int stat;
> +
> + stat = wait_for_completion_timeout(&i2c->cmp, HZ / 10);
> + if (!stat) {
> + /* If we can't unwedge, return timeout */
> + if (!dw_hdmi_i2c_unwedge(hdmi))
> + return -EAGAIN;
> +
> + /* We tried to unwedge; give it another chance */
> + stat = wait_for_completion_timeout(&i2c->cmp, HZ / 10);
> + if (!stat)
> + return -EAGAIN;
> + }
> +
> + /* Check for error condition on the bus */
> + if (i2c->stat & HDMI_IH_I2CM_STAT0_ERROR)
> + return -EIO;
> +
> + return 0;
> +}
> +
> static int dw_hdmi_i2c_read(struct dw_hdmi *hdmi,
> unsigned char *buf, unsigned int length)
> {
> struct dw_hdmi_i2c *i2c = hdmi->i2c;
> - int stat;
> + int ret;
>
> if (!i2c->is_regaddr) {
> dev_dbg(hdmi->dev, "set read register address to 0\n");
> @@ -270,13 +346,9 @@ static int dw_hdmi_i2c_read(struct dw_hdmi *hdmi,
> hdmi_writeb(hdmi, HDMI_I2CM_OPERATION_READ,
> HDMI_I2CM_OPERATION);
>
> - stat = wait_for_completion_timeout(&i2c->cmp, HZ / 10);
> - if (!stat)
> - return -EAGAIN;
> -
> - /* Check for error condition on the bus */
> - if (i2c->stat & HDMI_IH_I2CM_STAT0_ERROR)
> - return -EIO;
> + ret = dw_hdmi_i2c_wait(hdmi);
> + if (ret)
> + return ret;
>
> *buf++ = hdmi_readb(hdmi, HDMI_I2CM_DATAI);
> }
> @@ -289,7 +361,7 @@ static int dw_hdmi_i2c_write(struct dw_hdmi *hdmi,
> unsigned char *buf, unsigned int length)
> {
> struct dw_hdmi_i2c *i2c = hdmi->i2c;
> - int stat;
> + int ret;
>
> if (!i2c->is_regaddr) {
> /* Use the first write byte as register address */
> @@ -307,13 +379,9 @@ static int dw_hdmi_i2c_write(struct dw_hdmi *hdmi,
> hdmi_writeb(hdmi, HDMI_I2CM_OPERATION_WRITE,
> HDMI_I2CM_OPERATION);
>
> - stat = wait_for_completion_timeout(&i2c->cmp, HZ / 10);
> - if (!stat)
> - return -EAGAIN;
> -
> - /* Check for error condition on the bus */
> - if (i2c->stat & HDMI_IH_I2CM_STAT0_ERROR)
> - return -EIO;
> + ret = dw_hdmi_i2c_wait(hdmi);
> + if (ret)
> + return ret;
> }
>
> return 0;
> @@ -2606,6 +2674,22 @@ __dw_hdmi_probe(struct platform_device *pdev,
>
> /* If DDC bus is not specified, try to register HDMI I2C bus */
> if (!hdmi->ddc) {
> + /* Look for (optional) stuff related to unwedging */
> + hdmi->pinctrl = devm_pinctrl_get(dev);
> + if (!IS_ERR(hdmi->pinctrl)) {
> + hdmi->unwedge_state =
> + pinctrl_lookup_state(hdmi->pinctrl, "unwedge");
> + hdmi->default_state =
> + pinctrl_lookup_state(hdmi->pinctrl, "default");
> +
> + if (IS_ERR(hdmi->default_state) &&
> + !IS_ERR(hdmi->unwedge_state)) {
> + dev_warn(dev,
> + "Unwedge requires default pinctrl\n");
Can you downgrade this message to info or dbg? Given how rare this issue is, we
probably don't want to spam everyone who is happily using dw-hdmi.
With that,
Reviewed-by: Sean Paul <s...@poorly.run>
Sean
> + hdmi->unwedge_state = ERR_PTR(-ENODEV);
> + }
> + }
> +
> hdmi->ddc = dw_hdmi_i2c_adapter(hdmi);
> if (IS_ERR(hdmi->ddc))
> hdmi->ddc = NULL;
> --
> 2.21.0.1020.gf2820cf01a-goog
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
