| Message ID | 20200217105513.2.I477092c2f104fd589133436c3ae4590e6fc6323b@changeid (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC hdmi-codec: fix HDMI jack reporting | expand |
Hi, Tzhung-Bi: On Mon, 2020-02-17 at 11:16 +0800, Tzung-Bi Shih wrote: > hdmi_conn_detect and mtk_hdmi_audio_hook_plugged_cb would be called > by different threads. > > Imaging the following calling sequence: > Thread A Thread B > -------------------------------------------------------------------- > mtk_hdmi_audio_hook_plugged_cb() > mtk_cec_hpd_high() -> disconnected > hdmi_conn_detect() > mtk_cec_hpd_high() -> connected > plugged_cb(connected) > plugged_cb(disconnected) > > The latest disconnected is false reported. Makes mtk_cec_hpd_high > and plugged_cb atomic to fix. > > Also uses the same lock to protect read/write of plugged_cb and codec_dev. > > Fixes: 5d3c64477392 ("drm/mediatek: support HDMI jack status reporting") This patch looks good to me, but please merge this patch with the patch it fix. Regards, CK > Signed-off-by: Tzung-Bi Shih <tzungbi@google.com> > --- > drivers/gpu/drm/mediatek/mtk_hdmi.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c > index 03aeb73005ef..d80017e3d84a 100644 > --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c > +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c > @@ -12,6 +12,7 @@ > #include <linux/io.h> > #include <linux/kernel.h> > #include <linux/mfd/syscon.h> > +#include <linux/mutex.h> > #include <linux/of_platform.h> > #include <linux/of.h> > #include <linux/of_gpio.h> > @@ -171,6 +172,7 @@ struct mtk_hdmi { > bool enabled; > hdmi_codec_plugged_cb plugged_cb; > struct device *codec_dev; > + struct mutex update_plugged_status_lock; > }; > > static inline struct mtk_hdmi *hdmi_ctx_from_bridge(struct drm_bridge *b) > @@ -1199,10 +1201,13 @@ static void mtk_hdmi_clk_disable_audio(struct mtk_hdmi *hdmi) > static enum drm_connector_status > mtk_hdmi_update_plugged_status(struct mtk_hdmi *hdmi) > { > - bool connected = mtk_cec_hpd_high(hdmi->cec_dev); > + bool connected; > > + mutex_lock(&hdmi->update_plugged_status_lock); > + connected = mtk_cec_hpd_high(hdmi->cec_dev); > if (hdmi->plugged_cb && hdmi->codec_dev) > hdmi->plugged_cb(hdmi->codec_dev, connected); > + mutex_unlock(&hdmi->update_plugged_status_lock); > > return connected ? > connector_status_connected : connector_status_disconnected; > @@ -1669,8 +1674,11 @@ static int mtk_hdmi_audio_hook_plugged_cb(struct device *dev, void *data, > { > struct mtk_hdmi *hdmi = data; > > + mutex_lock(&hdmi->update_plugged_status_lock); > hdmi->plugged_cb = fn; > hdmi->codec_dev = codec_dev; > + mutex_unlock(&hdmi->update_plugged_status_lock); > + > mtk_hdmi_update_plugged_status(hdmi); > > return 0; > @@ -1729,6 +1737,7 @@ static int mtk_drm_hdmi_probe(struct platform_device *pdev) > return ret; > } > > + mutex_init(&hdmi->update_plugged_status_lock); > platform_set_drvdata(pdev, hdmi); > > ret = mtk_hdmi_output_init(hdmi);
On Mon, Feb 17, 2020 at 11:44 AM CK Hu <ck.hu@mediatek.com> wrote: > On Mon, 2020-02-17 at 11:16 +0800, Tzung-Bi Shih wrote: > > Fixes: 5d3c64477392 ("drm/mediatek: support HDMI jack status reporting") > > This patch looks good to me, but please merge this patch with the patch > it fix. 5d3c64477392 ("drm/mediatek: support HDMI jack status reporting") has applied to ASoC for-next branch. This is a fixup patch.
On Mon, 2020-02-17 at 11:55 +0800, Tzung-Bi Shih wrote: > On Mon, Feb 17, 2020 at 11:44 AM CK Hu <ck.hu@mediatek.com> wrote: > > On Mon, 2020-02-17 at 11:16 +0800, Tzung-Bi Shih wrote: > > > Fixes: 5d3c64477392 ("drm/mediatek: support HDMI jack status reporting") > > > > This patch looks good to me, but please merge this patch with the patch > > it fix. > > 5d3c64477392 ("drm/mediatek: support HDMI jack status reporting") has > applied to ASoC for-next branch. This is a fixup patch. I thought a patch need an ack by the maintainer. OK, so I could do is, for this patch, Acked-by: CK Hu <ck.hu@mediatek.com>
diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c index 03aeb73005ef..d80017e3d84a 100644 --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c @@ -12,6 +12,7 @@ #include <linux/io.h> #include <linux/kernel.h> #include <linux/mfd/syscon.h> +#include <linux/mutex.h> #include <linux/of_platform.h> #include <linux/of.h> #include <linux/of_gpio.h> @@ -171,6 +172,7 @@ struct mtk_hdmi { bool enabled; hdmi_codec_plugged_cb plugged_cb; struct device *codec_dev; + struct mutex update_plugged_status_lock; }; static inline struct mtk_hdmi *hdmi_ctx_from_bridge(struct drm_bridge *b) @@ -1199,10 +1201,13 @@ static void mtk_hdmi_clk_disable_audio(struct mtk_hdmi *hdmi) static enum drm_connector_status mtk_hdmi_update_plugged_status(struct mtk_hdmi *hdmi) { - bool connected = mtk_cec_hpd_high(hdmi->cec_dev); + bool connected; + mutex_lock(&hdmi->update_plugged_status_lock); + connected = mtk_cec_hpd_high(hdmi->cec_dev); if (hdmi->plugged_cb && hdmi->codec_dev) hdmi->plugged_cb(hdmi->codec_dev, connected); + mutex_unlock(&hdmi->update_plugged_status_lock); return connected ? connector_status_connected : connector_status_disconnected; @@ -1669,8 +1674,11 @@ static int mtk_hdmi_audio_hook_plugged_cb(struct device *dev, void *data, { struct mtk_hdmi *hdmi = data; + mutex_lock(&hdmi->update_plugged_status_lock); hdmi->plugged_cb = fn; hdmi->codec_dev = codec_dev; + mutex_unlock(&hdmi->update_plugged_status_lock); + mtk_hdmi_update_plugged_status(hdmi); return 0; @@ -1729,6 +1737,7 @@ static int mtk_drm_hdmi_probe(struct platform_device *pdev) return ret; } + mutex_init(&hdmi->update_plugged_status_lock); platform_set_drvdata(pdev, hdmi); ret = mtk_hdmi_output_init(hdmi);
hdmi_conn_detect and mtk_hdmi_audio_hook_plugged_cb would be called by different threads. Imaging the following calling sequence: Thread A Thread B -------------------------------------------------------------------- mtk_hdmi_audio_hook_plugged_cb() mtk_cec_hpd_high() -> disconnected hdmi_conn_detect() mtk_cec_hpd_high() -> connected plugged_cb(connected) plugged_cb(disconnected) The latest disconnected is false reported. Makes mtk_cec_hpd_high and plugged_cb atomic to fix. Also uses the same lock to protect read/write of plugged_cb and codec_dev. Fixes: 5d3c64477392 ("drm/mediatek: support HDMI jack status reporting") Signed-off-by: Tzung-Bi Shih <tzungbi@google.com> --- drivers/gpu/drm/mediatek/mtk_hdmi.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)