| Message ID | 27380b83a1b17419a35a13e29a86a1989204d1c3.1649230434.git.Sandor.yu@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DRM: Bridge: DW_HDMI: Add new features and bug fix | expand |
Hi Sandor, On 06/04/2022 10:48, Sandor.yu@nxp.com wrote: > From: Sandor Yu <Sandor.yu@nxp.com> > > CEC interrupt status/mask and logical address registers > will be reset when device enter suspend. > It will cause cec fail to work after device resume. > Add CEC suspend/resume functions, save these registers status > when suspend and restore them when resume. > > Signed-off-by: Sandor Yu <Sandor.yu@nxp.com> > --- > drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c | 35 +++++++++++++++++++ > 1 file changed, 35 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c > index c8f44bcb298a..ceb619b32fde 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c > @@ -18,6 +18,8 @@ > > #include "dw-hdmi-cec.h" > > +static u8 cec_saved_regs[5]; Ewww, not a global. Just save it in struct dw_hdmi_cec. > + > enum { > HDMI_IH_CEC_STAT0 = 0x0106, > HDMI_IH_MUTE_CEC_STAT0 = 0x0186, > @@ -306,11 +308,44 @@ static int dw_hdmi_cec_remove(struct platform_device *pdev) > return 0; > } > > +static int __maybe_unused dw_hdmi_cec_resume(struct device *dev) > +{ > + struct dw_hdmi_cec *cec = dev_get_drvdata(dev); > + > + /* Restore logical address and interrupt status/mask register */ > + dw_hdmi_write(cec, cec_saved_regs[0], HDMI_CEC_ADDR_L); > + dw_hdmi_write(cec, cec_saved_regs[1], HDMI_CEC_ADDR_H); No need to store HDMI_CEC_ADDR_L/R, just use cec->addresses. > + dw_hdmi_write(cec, cec_saved_regs[2], HDMI_CEC_POLARITY); > + dw_hdmi_write(cec, cec_saved_regs[3], HDMI_CEC_MASK); > + dw_hdmi_write(cec, cec_saved_regs[4], HDMI_IH_MUTE_CEC_STAT0); This doesn't call cec->ops->disable/enable. It might be better to just call dw_hdmi_cec_enable() and store the enable state in struct dw_hdmi_cec. Regards, Hans > + > + return 0; > +} > + > +static int __maybe_unused dw_hdmi_cec_suspend(struct device *dev) > +{ > + struct dw_hdmi_cec *cec = dev_get_drvdata(dev); > + > + /* store logical address and interrupt status/mask register */ > + cec_saved_regs[0] = dw_hdmi_read(cec, HDMI_CEC_ADDR_L); > + cec_saved_regs[1] = dw_hdmi_read(cec, HDMI_CEC_ADDR_H); > + cec_saved_regs[2] = dw_hdmi_read(cec, HDMI_CEC_POLARITY); > + cec_saved_regs[3] = dw_hdmi_read(cec, HDMI_CEC_MASK); > + cec_saved_regs[4] = dw_hdmi_read(cec, HDMI_IH_MUTE_CEC_STAT0); > + > + return 0; > +} > + > +static const struct dev_pm_ops dw_hdmi_cec_pm = { > + SET_SYSTEM_SLEEP_PM_OPS(dw_hdmi_cec_suspend, dw_hdmi_cec_resume) > +}; > + > static struct platform_driver dw_hdmi_cec_driver = { > .probe = dw_hdmi_cec_probe, > .remove = dw_hdmi_cec_remove, > .driver = { > .name = "dw-hdmi-cec", > + .pm = &dw_hdmi_cec_pm, > }, > }; > module_platform_driver(dw_hdmi_cec_driver);
Hi, > -----Original Message----- > From: Hans Verkuil <hverkuil-cisco@xs4all.nl> > Sent: 2022年4月6日 17:47 > To: Sandor Yu <sandor.yu@nxp.com>; dri-devel@lists.freedesktop.org; > linux-kernel@vger.kernel.org; andrzej.hajda@intel.com; > narmstrong@baylibre.com; robert.foss@linaro.org; > Laurent.pinchart@ideasonboard.com; jonas@kwiboo.se; > jernej.skrabec@gmail.com > Cc: S.J. Wang <shengjiu.wang@nxp.com>; cai.huoqing@linux.dev; > maxime@cerno.tech; harry.wentland@amd.com; amuel@sholland.org > Subject: [EXT] Re: [PATCH v1 1/5] drm: bridge: dw_hdmi: cec: Add cec > suspend/resume function > > Caution: EXT Email > > Hi Sandor, > > On 06/04/2022 10:48, Sandor.yu@nxp.com wrote: > > From: Sandor Yu <Sandor.yu@nxp.com> > > > > CEC interrupt status/mask and logical address registers will be reset > > when device enter suspend. > > It will cause cec fail to work after device resume. > > Add CEC suspend/resume functions, save these registers status when > > suspend and restore them when resume. > > > > Signed-off-by: Sandor Yu <Sandor.yu@nxp.com> > > --- > > drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c | 35 > > +++++++++++++++++++ > > 1 file changed, 35 insertions(+) > > > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c > > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c > > index c8f44bcb298a..ceb619b32fde 100644 > > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c > > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c > > @@ -18,6 +18,8 @@ > > > > #include "dw-hdmi-cec.h" > > > > +static u8 cec_saved_regs[5]; > > Ewww, not a global. Just save it in struct dw_hdmi_cec. > > > + > > enum { > > HDMI_IH_CEC_STAT0 = 0x0106, > > HDMI_IH_MUTE_CEC_STAT0 = 0x0186, @@ -306,11 +308,44 > @@ static > > int dw_hdmi_cec_remove(struct platform_device *pdev) > > return 0; > > } > > > > +static int __maybe_unused dw_hdmi_cec_resume(struct device *dev) { > > + struct dw_hdmi_cec *cec = dev_get_drvdata(dev); > > + > > + /* Restore logical address and interrupt status/mask register */ > > + dw_hdmi_write(cec, cec_saved_regs[0], HDMI_CEC_ADDR_L); > > + dw_hdmi_write(cec, cec_saved_regs[1], HDMI_CEC_ADDR_H); > > No need to store HDMI_CEC_ADDR_L/R, just use cec->addresses. Agree, I will use it later. > > > + dw_hdmi_write(cec, cec_saved_regs[2], HDMI_CEC_POLARITY); > > + dw_hdmi_write(cec, cec_saved_regs[3], HDMI_CEC_MASK); > > + dw_hdmi_write(cec, cec_saved_regs[4], > HDMI_IH_MUTE_CEC_STAT0); > > This doesn't call cec->ops->disable/enable. It might be better to just call > dw_hdmi_cec_enable() and store the enable state in struct dw_hdmi_cec. If call cec->ops->disable/enable in suspend/resume, CEC stack will be reset. CEC user such as EARC/ARC, the CEC link is expect keep in active after device resume. So I prefer only save those registers for better user experience. > > Regards, > > Hans > Thanks Sandor > > + > > + return 0; > > +} > > + > > +static int __maybe_unused dw_hdmi_cec_suspend(struct device *dev) { > > + struct dw_hdmi_cec *cec = dev_get_drvdata(dev); > > + > > + /* store logical address and interrupt status/mask register */ > > + cec_saved_regs[0] = dw_hdmi_read(cec, HDMI_CEC_ADDR_L); > > + cec_saved_regs[1] = dw_hdmi_read(cec, HDMI_CEC_ADDR_H); > > + cec_saved_regs[2] = dw_hdmi_read(cec, HDMI_CEC_POLARITY); > > + cec_saved_regs[3] = dw_hdmi_read(cec, HDMI_CEC_MASK); > > + cec_saved_regs[4] = dw_hdmi_read(cec, > HDMI_IH_MUTE_CEC_STAT0); > > + > > + return 0; > > +} > > + > > +static const struct dev_pm_ops dw_hdmi_cec_pm = { > > + SET_SYSTEM_SLEEP_PM_OPS(dw_hdmi_cec_suspend, > dw_hdmi_cec_resume) > > +}; > > + > > static struct platform_driver dw_hdmi_cec_driver = { > > .probe = dw_hdmi_cec_probe, > > .remove = dw_hdmi_cec_remove, > > .driver = { > > .name = "dw-hdmi-cec", > > + .pm = &dw_hdmi_cec_pm, > > }, > > }; > > module_platform_driver(dw_hdmi_cec_driver);
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c index c8f44bcb298a..ceb619b32fde 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c @@ -18,6 +18,8 @@ #include "dw-hdmi-cec.h" +static u8 cec_saved_regs[5]; + enum { HDMI_IH_CEC_STAT0 = 0x0106, HDMI_IH_MUTE_CEC_STAT0 = 0x0186, @@ -306,11 +308,44 @@ static int dw_hdmi_cec_remove(struct platform_device *pdev) return 0; } +static int __maybe_unused dw_hdmi_cec_resume(struct device *dev) +{ + struct dw_hdmi_cec *cec = dev_get_drvdata(dev); + + /* Restore logical address and interrupt status/mask register */ + dw_hdmi_write(cec, cec_saved_regs[0], HDMI_CEC_ADDR_L); + dw_hdmi_write(cec, cec_saved_regs[1], HDMI_CEC_ADDR_H); + dw_hdmi_write(cec, cec_saved_regs[2], HDMI_CEC_POLARITY); + dw_hdmi_write(cec, cec_saved_regs[3], HDMI_CEC_MASK); + dw_hdmi_write(cec, cec_saved_regs[4], HDMI_IH_MUTE_CEC_STAT0); + + return 0; +} + +static int __maybe_unused dw_hdmi_cec_suspend(struct device *dev) +{ + struct dw_hdmi_cec *cec = dev_get_drvdata(dev); + + /* store logical address and interrupt status/mask register */ + cec_saved_regs[0] = dw_hdmi_read(cec, HDMI_CEC_ADDR_L); + cec_saved_regs[1] = dw_hdmi_read(cec, HDMI_CEC_ADDR_H); + cec_saved_regs[2] = dw_hdmi_read(cec, HDMI_CEC_POLARITY); + cec_saved_regs[3] = dw_hdmi_read(cec, HDMI_CEC_MASK); + cec_saved_regs[4] = dw_hdmi_read(cec, HDMI_IH_MUTE_CEC_STAT0); + + return 0; +} + +static const struct dev_pm_ops dw_hdmi_cec_pm = { + SET_SYSTEM_SLEEP_PM_OPS(dw_hdmi_cec_suspend, dw_hdmi_cec_resume) +}; + static struct platform_driver dw_hdmi_cec_driver = { .probe = dw_hdmi_cec_probe, .remove = dw_hdmi_cec_remove, .driver = { .name = "dw-hdmi-cec", + .pm = &dw_hdmi_cec_pm, }, }; module_platform_driver(dw_hdmi_cec_driver);