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@xxxxxxxxxx> > --- > 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); _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel