[PATCH] omapdrm: hdmi4_cec: Fix CEC clock handling for PM

[PATCH] omapdrm: hdmi4_cec: Fix CEC clock handling for PM

[Tony Lindgren]

If CONFIG_OMAP4_DSS_HDMI_CEC is enabled in .config, deeper SoC idle
states are blocked because the CEC clock gets always enabled on init.

Let's fix the issue by moving the CEC clock handling to happen later in
hdmi_cec_adap_enable() as suggested by Hans Verkuil <hverkuil@xs4all.nl>.
This way the CEC clock gets only enabled when needed. This can be tested
by doing cec-ctl --playback to enable the CEC, and doing cec-ctl --clear
to disable it.

Let's also fix the typo for "divider" in the comments while at it.

Fixes: 8d7f934df8d8 ("omapdrm: hdmi4_cec: add OMAP4 HDMI CEC support")
Suggested-by: Hans Verkuil <hverkuil@xs4all.nl>
Cc: Hans Verkuil <hverkuil@xs4all.nl>
Cc: Jyri Sarha <jsarha@ti.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
@@ -169,12 +169,19 @@ static int hdmi_cec_adap_enable(struct cec_adapter *adap, bool enable)
 	struct hdmi_core_data *core = cec_get_drvdata(adap);
 	int temp, err;
 
-	if (!enable) {
+	if (enable) {
+		/*
+		 * Initialize CEC clock divider: CEC needs 2MHz clock hence
+		 * set the divider to 24 to get 48/24=2MHz clock
+		 */
+		REG_FLD_MOD(core->wp->base, HDMI_WP_CLK, 0x18, 5, 0);
+	} else {
 		hdmi_write_reg(core->base, HDMI_CEC_INT_ENABLE_0, 0);
 		hdmi_write_reg(core->base, HDMI_CEC_INT_ENABLE_1, 0);
 		REG_FLD_MOD(core->base, HDMI_CORE_SYS_INTR_UNMASK4, 0, 3, 3);
 		hdmi_wp_clear_irqenable(core->wp, HDMI_IRQ_CORE);
 		hdmi_wp_set_irqstatus(core->wp, HDMI_IRQ_CORE);
+		REG_FLD_MOD(core->wp->base, HDMI_WP_CLK, 0, 5, 0);
 		hdmi4_core_disable(core);
 		return 0;
 	}
@@ -333,11 +340,8 @@ int hdmi4_cec_init(struct platform_device *pdev, struct hdmi_core_data *core,
 		return ret;
 	core->wp = wp;
 
-	/*
-	 * Initialize CEC clock divider: CEC needs 2MHz clock hence
-	 * set the devider to 24 to get 48/24=2MHz clock
-	 */
-	REG_FLD_MOD(core->wp->base, HDMI_WP_CLK, 0x18, 5, 0);
+	/* Disable clock initially, hdmi_cec_adap_enable() manages it */
+	REG_FLD_MOD(core->wp->base, HDMI_WP_CLK, 0, 5, 0);
 
 	ret = cec_register_adapter(core->adap, &pdev->dev);
 	if (ret < 0) {
-- 
2.21.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] omapdrm: hdmi4_cec: Fix CEC clock handling for PM

[Laurent Pinchart]

Hi Tony,

Thank you for the patch.

On Mon, Mar 25, 2019 at 04:47:43PM -0700, Tony Lindgren wrote:
> If CONFIG_OMAP4_DSS_HDMI_CEC is enabled in .config, deeper SoC idle
> states are blocked because the CEC clock gets always enabled on init.
> 
> Let's fix the issue by moving the CEC clock handling to happen later in
> hdmi_cec_adap_enable() as suggested by Hans Verkuil <hverkuil@xs4all.nl>.
> This way the CEC clock gets only enabled when needed. This can be tested
> by doing cec-ctl --playback to enable the CEC, and doing cec-ctl --clear
> to disable it.
> 
> Let's also fix the typo for "divider" in the comments while at it.
> 
> Fixes: 8d7f934df8d8 ("omapdrm: hdmi4_cec: add OMAP4 HDMI CEC support")
> Suggested-by: Hans Verkuil <hverkuil@xs4all.nl>
> Cc: Hans Verkuil <hverkuil@xs4all.nl>
> Cc: Jyri Sarha <jsarha@ti.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
> @@ -169,12 +169,19 @@ static int hdmi_cec_adap_enable(struct cec_adapter *adap, bool enable)
>  	struct hdmi_core_data *core = cec_get_drvdata(adap);
>  	int temp, err;
>  
> -	if (!enable) {
> +	if (enable) {
> +		/*
> +		 * Initialize CEC clock divider: CEC needs 2MHz clock hence
> +		 * set the divider to 24 to get 48/24=2MHz clock
> +		 */
> +		REG_FLD_MOD(core->wp->base, HDMI_WP_CLK, 0x18, 5, 0);
> +	} else {
>  		hdmi_write_reg(core->base, HDMI_CEC_INT_ENABLE_0, 0);
>  		hdmi_write_reg(core->base, HDMI_CEC_INT_ENABLE_1, 0);
>  		REG_FLD_MOD(core->base, HDMI_CORE_SYS_INTR_UNMASK4, 0, 3, 3);
>  		hdmi_wp_clear_irqenable(core->wp, HDMI_IRQ_CORE);
>  		hdmi_wp_set_irqstatus(core->wp, HDMI_IRQ_CORE);
> +		REG_FLD_MOD(core->wp->base, HDMI_WP_CLK, 0, 5, 0);
>  		hdmi4_core_disable(core);
>  		return 0;
>  	}
> @@ -333,11 +340,8 @@ int hdmi4_cec_init(struct platform_device *pdev, struct hdmi_core_data *core,
>  		return ret;
>  	core->wp = wp;
>  
> -	/*
> -	 * Initialize CEC clock divider: CEC needs 2MHz clock hence
> -	 * set the devider to 24 to get 48/24=2MHz clock
> -	 */
> -	REG_FLD_MOD(core->wp->base, HDMI_WP_CLK, 0x18, 5, 0);
> +	/* Disable clock initially, hdmi_cec_adap_enable() manages it */
> +	REG_FLD_MOD(core->wp->base, HDMI_WP_CLK, 0, 5, 0);
>  
>  	ret = cec_register_adapter(core->adap, &pdev->dev);
>  	if (ret < 0) {

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] omapdrm: hdmi4_cec: Fix CEC clock handling for PM

[Hans Verkuil]

On 3/26/19 12:47 AM, Tony Lindgren wrote:
> If CONFIG_OMAP4_DSS_HDMI_CEC is enabled in .config, deeper SoC idle
> states are blocked because the CEC clock gets always enabled on init.
> 
> Let's fix the issue by moving the CEC clock handling to happen later in
> hdmi_cec_adap_enable() as suggested by Hans Verkuil <hverkuil@xs4all.nl>.
> This way the CEC clock gets only enabled when needed. This can be tested
> by doing cec-ctl --playback to enable the CEC, and doing cec-ctl --clear
> to disable it.
> 
> Let's also fix the typo for "divider" in the comments while at it.
> 
> Fixes: 8d7f934df8d8 ("omapdrm: hdmi4_cec: add OMAP4 HDMI CEC support")
> Suggested-by: Hans Verkuil <hverkuil@xs4all.nl>
> Cc: Hans Verkuil <hverkuil@xs4all.nl>
> Cc: Jyri Sarha <jsarha@ti.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>  drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
> @@ -169,12 +169,19 @@ static int hdmi_cec_adap_enable(struct cec_adapter *adap, bool enable)
>  	struct hdmi_core_data *core = cec_get_drvdata(adap);
>  	int temp, err;
>  
> -	if (!enable) {
> +	if (enable) {
> +		/*
> +		 * Initialize CEC clock divider: CEC needs 2MHz clock hence
> +		 * set the divider to 24 to get 48/24=2MHz clock
> +		 */
> +		REG_FLD_MOD(core->wp->base, HDMI_WP_CLK, 0x18, 5, 0);

Please move this to after the 'if'. That's where all the other
'enable' code is.

Regards,

	Hans

> +	} else {
>  		hdmi_write_reg(core->base, HDMI_CEC_INT_ENABLE_0, 0);
>  		hdmi_write_reg(core->base, HDMI_CEC_INT_ENABLE_1, 0);
>  		REG_FLD_MOD(core->base, HDMI_CORE_SYS_INTR_UNMASK4, 0, 3, 3);
>  		hdmi_wp_clear_irqenable(core->wp, HDMI_IRQ_CORE);
>  		hdmi_wp_set_irqstatus(core->wp, HDMI_IRQ_CORE);
> +		REG_FLD_MOD(core->wp->base, HDMI_WP_CLK, 0, 5, 0);
>  		hdmi4_core_disable(core);
>  		return 0;
>  	}
> @@ -333,11 +340,8 @@ int hdmi4_cec_init(struct platform_device *pdev, struct hdmi_core_data *core,
>  		return ret;
>  	core->wp = wp;
>  
> -	/*
> -	 * Initialize CEC clock divider: CEC needs 2MHz clock hence
> -	 * set the devider to 24 to get 48/24=2MHz clock
> -	 */
> -	REG_FLD_MOD(core->wp->base, HDMI_WP_CLK, 0x18, 5, 0);
> +	/* Disable clock initially, hdmi_cec_adap_enable() manages it */
> +	REG_FLD_MOD(core->wp->base, HDMI_WP_CLK, 0, 5, 0);
>  
>  	ret = cec_register_adapter(core->adap, &pdev->dev);
>  	if (ret < 0) {
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] omapdrm: hdmi4_cec: Fix CEC clock handling for PM

[Tony Lindgren]

* Hans Verkuil <hverkuil@xs4all.nl> [190326 06:36]:
> On 3/26/19 12:47 AM, Tony Lindgren wrote:
> > @@ -169,12 +169,19 @@ static int hdmi_cec_adap_enable(struct cec_adapter *adap, bool enable)
> >  	struct hdmi_core_data *core = cec_get_drvdata(adap);
> >  	int temp, err;
> >  
> > -	if (!enable) {
> > +	if (enable) {
> > +		/*
> > +		 * Initialize CEC clock divider: CEC needs 2MHz clock hence
> > +		 * set the divider to 24 to get 48/24=2MHz clock
> > +		 */
> > +		REG_FLD_MOD(core->wp->base, HDMI_WP_CLK, 0x18, 5, 0);
> 
> Please move this to after the 'if'. That's where all the other
> 'enable' code is.

Oops yeah that's better, will post v2 shortly.

Regards,

Tony
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel