[PATCH 0/3] Enable CEC on rk3399

[PATCH 0/3] Enable CEC on rk3399

[Pierre-Hugues Husson]

Enable CEC on firefly-rk3399.
Tested on a TV with cec-ctl --playback; cec-ctl -S

Pierre-Hugues Husson (3):
  drm: bridge: synopsys/dw-hdmi: Enable cec clock
  arm64: dts: rockchip: add the cec clk for dw-mipi-hdmi on rk3399
  arm64: dts: rockchip: enable cec pin for rk3399 firefly

 arch/arm64/boot/dts/rockchip/rk3399-firefly.dts |  2 ++
 arch/arm64/boot/dts/rockchip/rk3399.dtsi        |  8 ++++++--
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c       | 16 ++++++++++++++++
 3 files changed, 24 insertions(+), 2 deletions(-)

-- 
2.14.1

[PATCH 0/3] Enable CEC on rk3399

[Pierre-Hugues Husson]

Enable CEC on firefly-rk3399.
Tested on a TV with cec-ctl --playback; cec-ctl -S

Pierre-Hugues Husson (3):
  drm: bridge: synopsys/dw-hdmi: Enable cec clock
  arm64: dts: rockchip: add the cec clk for dw-mipi-hdmi on rk3399
  arm64: dts: rockchip: enable cec pin for rk3399 firefly

 arch/arm64/boot/dts/rockchip/rk3399-firefly.dts |  2 ++
 arch/arm64/boot/dts/rockchip/rk3399.dtsi        |  8 ++++++--
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c       | 16 ++++++++++++++++
 3 files changed, 24 insertions(+), 2 deletions(-)

-- 
2.14.1

[PATCH 1/3] drm: bridge: synopsys/dw-hdmi: Enable cec clock

[Pierre-Hugues Husson]

The documentation already mentions "cec" optional clock, but
currently the driver doesn't enable it.

Signed-off-by: Pierre-Hugues Husson <phh@phh.me>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index bf14214fa464..5007cdf43131 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -138,6 +138,7 @@ struct dw_hdmi {
 	struct device *dev;
 	struct clk *isfr_clk;
 	struct clk *iahb_clk;
+	struct clk *cec_clk;
 	struct dw_hdmi_i2c *i2c;
 
 	struct hdmi_data_info hdmi_data;
@@ -2382,6 +2383,18 @@ __dw_hdmi_probe(struct platform_device *pdev,
 		goto err_isfr;
 	}
 
+	hdmi->cec_clk = devm_clk_get(hdmi->dev, "cec");
+	if (IS_ERR(hdmi->cec_clk)) {
+		hdmi->cec_clk = NULL;
+	} else {
+		ret = clk_prepare_enable(hdmi->cec_clk);
+		if (ret) {
+			dev_err(hdmi->dev, "Cannot enable HDMI cec clock: %d\n",
+					ret);
+			goto err_res;
+		}
+	}
+
 	/* Product and revision IDs */
 	hdmi->version = (hdmi_readb(hdmi, HDMI_DESIGN_ID) << 8)
 		      | (hdmi_readb(hdmi, HDMI_REVISION_ID) << 0);
@@ -2518,6 +2531,8 @@ __dw_hdmi_probe(struct platform_device *pdev,
 		cec_notifier_put(hdmi->cec_notifier);
 
 	clk_disable_unprepare(hdmi->iahb_clk);
+	if (hdmi->cec_clk)
+		clk_disable_unprepare(hdmi->cec_clk);
 err_isfr:
 	clk_disable_unprepare(hdmi->isfr_clk);
 err_res:
@@ -2541,6 +2556,8 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi)
 
 	clk_disable_unprepare(hdmi->iahb_clk);
 	clk_disable_unprepare(hdmi->isfr_clk);
+	if (hdmi->cec_clk)
+		clk_disable_unprepare(hdmi->cec_clk);
 
 	if (hdmi->i2c)
 		i2c_del_adapter(&hdmi->i2c->adap);
-- 
2.14.1

[PATCH 1/3] drm: bridge: synopsys/dw-hdmi: Enable cec clock

[Pierre-Hugues Husson]

The documentation already mentions "cec" optional clock, but
currently the driver doesn't enable it.

Signed-off-by: Pierre-Hugues Husson <phh@phh.me>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index bf14214fa464..5007cdf43131 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -138,6 +138,7 @@ struct dw_hdmi {
 	struct device *dev;
 	struct clk *isfr_clk;
 	struct clk *iahb_clk;
+	struct clk *cec_clk;
 	struct dw_hdmi_i2c *i2c;
 
 	struct hdmi_data_info hdmi_data;
@@ -2382,6 +2383,18 @@ __dw_hdmi_probe(struct platform_device *pdev,
 		goto err_isfr;
 	}
 
+	hdmi->cec_clk = devm_clk_get(hdmi->dev, "cec");
+	if (IS_ERR(hdmi->cec_clk)) {
+		hdmi->cec_clk = NULL;
+	} else {
+		ret = clk_prepare_enable(hdmi->cec_clk);
+		if (ret) {
+			dev_err(hdmi->dev, "Cannot enable HDMI cec clock: %d\n",
+					ret);
+			goto err_res;
+		}
+	}
+
 	/* Product and revision IDs */
 	hdmi->version = (hdmi_readb(hdmi, HDMI_DESIGN_ID) << 8)
 		      | (hdmi_readb(hdmi, HDMI_REVISION_ID) << 0);
@@ -2518,6 +2531,8 @@ __dw_hdmi_probe(struct platform_device *pdev,
 		cec_notifier_put(hdmi->cec_notifier);
 
 	clk_disable_unprepare(hdmi->iahb_clk);
+	if (hdmi->cec_clk)
+		clk_disable_unprepare(hdmi->cec_clk);
 err_isfr:
 	clk_disable_unprepare(hdmi->isfr_clk);
 err_res:
@@ -2541,6 +2556,8 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi)
 
 	clk_disable_unprepare(hdmi->iahb_clk);
 	clk_disable_unprepare(hdmi->isfr_clk);
+	if (hdmi->cec_clk)
+		clk_disable_unprepare(hdmi->cec_clk);
 
 	if (hdmi->i2c)
 		i2c_del_adapter(&hdmi->i2c->adap);
-- 
2.14.1

Re: [PATCH 1/3] drm: bridge: synopsys/dw-hdmi: Enable cec clock

[Fabio Estevam]

Hi Pierre-Hugues,

On Fri, Oct 13, 2017 at 7:53 PM, Pierre-Hugues Husson <phh@phh.me> wrote:
> The documentation already mentions "cec" optional clock, but
> currently the driver doesn't enable it.

The cec clock is enabled at dw_hdmi_cec_enable().

[PATCH 1/3] drm: bridge: synopsys/dw-hdmi: Enable cec clock

[Fabio Estevam]

Hi Pierre-Hugues,

On Fri, Oct 13, 2017 at 7:53 PM, Pierre-Hugues Husson <phh@phh.me> wrote:
> The documentation already mentions "cec" optional clock, but
> currently the driver doesn't enable it.

The cec clock is enabled at dw_hdmi_cec_enable().

Re: [PATCH 1/3] drm: bridge: synopsys/dw-hdmi: Enable cec clock

[Pierre-Hugues Husson]

Hi Fabio,

2017-10-14 2:18 GMT+02:00 Fabio Estevam <festevam@gmail.com>:
> Hi Pierre-Hugues,
>
> On Fri, Oct 13, 2017 at 7:53 PM, Pierre-Hugues Husson <phh@phh.me> wrote:
>> The documentation already mentions "cec" optional clock, but
>> currently the driver doesn't enable it.
>
> The cec clock is enabled at dw_hdmi_cec_enable().
As far as I understand, dw_hdmi_cec_enable only gates
the CEC clock inside the Synopsis IP,
but the SOC still has to provide a specific CEC clock to it.
To enable such an external CEC clock, the binding documentation [1]
mentions a "cec" optional clock, and I'm not seeing any code in dw-hdmi
to enable it.

Regards,

[1] On next-20170929
Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt line 28

[PATCH 1/3] drm: bridge: synopsys/dw-hdmi: Enable cec clock

[Pierre-Hugues Husson]

Hi Fabio,

2017-10-14 2:18 GMT+02:00 Fabio Estevam <festevam@gmail.com>:
> Hi Pierre-Hugues,
>
> On Fri, Oct 13, 2017 at 7:53 PM, Pierre-Hugues Husson <phh@phh.me> wrote:
>> The documentation already mentions "cec" optional clock, but
>> currently the driver doesn't enable it.
>
> The cec clock is enabled at dw_hdmi_cec_enable().
As far as I understand, dw_hdmi_cec_enable only gates
the CEC clock inside the Synopsis IP,
but the SOC still has to provide a specific CEC clock to it.
To enable such an external CEC clock, the binding documentation [1]
mentions a "cec" optional clock, and I'm not seeing any code in dw-hdmi
to enable it.

Regards,

[1] On next-20170929
Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt line 28

Re: [PATCH 1/3] drm: bridge: synopsys/dw-hdmi: Enable cec clock

[Fabio Estevam]

Hi Pierre,

On Sat, Oct 14, 2017 at 10:21 AM, Pierre-Hugues Husson <phh@phh.me> wrote:

> As far as I understand, dw_hdmi_cec_enable only gates
> the CEC clock inside the Synopsis IP,
> but the SOC still has to provide a specific CEC clock to it.
> To enable such an external CEC clock, the binding documentation [1]
> mentions a "cec" optional clock, and I'm not seeing any code in dw-hdmi
> to enable it.

Ok, understood.

In the case of i.MX6 we control the CEC clock via
HDMI_MC_CLKDIS_CECCLK_DISABLE bit inside the Synopsis IP.

Looks like Rockchip needs the external CEC clock then.

Regards,

Fabio Estevam

[PATCH 1/3] drm: bridge: synopsys/dw-hdmi: Enable cec clock

[Fabio Estevam]

Hi Pierre,

On Sat, Oct 14, 2017 at 10:21 AM, Pierre-Hugues Husson <phh@phh.me> wrote:

> As far as I understand, dw_hdmi_cec_enable only gates
> the CEC clock inside the Synopsis IP,
> but the SOC still has to provide a specific CEC clock to it.
> To enable such an external CEC clock, the binding documentation [1]
> mentions a "cec" optional clock, and I'm not seeing any code in dw-hdmi
> to enable it.

Ok, understood.

In the case of i.MX6 we control the CEC clock via
HDMI_MC_CLKDIS_CECCLK_DISABLE bit inside the Synopsis IP.

Looks like Rockchip needs the external CEC clock then.

Regards,

Fabio Estevam

Re: [PATCH 1/3] drm: bridge: synopsys/dw-hdmi: Enable cec clock

[Hans Verkuil]

On 10/14/2017 12:53 AM, Pierre-Hugues Husson wrote:
> The documentation already mentions "cec" optional clock, but
> currently the driver doesn't enable it.
> 
> Signed-off-by: Pierre-Hugues Husson <phh@phh.me>

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Thanks!

	Hans

> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index bf14214fa464..5007cdf43131 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -138,6 +138,7 @@ struct dw_hdmi {
>  	struct device *dev;
>  	struct clk *isfr_clk;
>  	struct clk *iahb_clk;
> +	struct clk *cec_clk;
>  	struct dw_hdmi_i2c *i2c;
>  
>  	struct hdmi_data_info hdmi_data;
> @@ -2382,6 +2383,18 @@ __dw_hdmi_probe(struct platform_device *pdev,
>  		goto err_isfr;
>  	}
>  
> +	hdmi->cec_clk = devm_clk_get(hdmi->dev, "cec");
> +	if (IS_ERR(hdmi->cec_clk)) {
> +		hdmi->cec_clk = NULL;
> +	} else {
> +		ret = clk_prepare_enable(hdmi->cec_clk);
> +		if (ret) {
> +			dev_err(hdmi->dev, "Cannot enable HDMI cec clock: %d\n",
> +					ret);
> +			goto err_res;
> +		}
> +	}
> +
>  	/* Product and revision IDs */
>  	hdmi->version = (hdmi_readb(hdmi, HDMI_DESIGN_ID) << 8)
>  		      | (hdmi_readb(hdmi, HDMI_REVISION_ID) << 0);
> @@ -2518,6 +2531,8 @@ __dw_hdmi_probe(struct platform_device *pdev,
>  		cec_notifier_put(hdmi->cec_notifier);
>  
>  	clk_disable_unprepare(hdmi->iahb_clk);
> +	if (hdmi->cec_clk)
> +		clk_disable_unprepare(hdmi->cec_clk);
>  err_isfr:
>  	clk_disable_unprepare(hdmi->isfr_clk);
>  err_res:
> @@ -2541,6 +2556,8 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi)
>  
>  	clk_disable_unprepare(hdmi->iahb_clk);
>  	clk_disable_unprepare(hdmi->isfr_clk);
> +	if (hdmi->cec_clk)
> +		clk_disable_unprepare(hdmi->cec_clk);
>  
>  	if (hdmi->i2c)
>  		i2c_del_adapter(&hdmi->i2c->adap);
> 

[PATCH 1/3] drm: bridge: synopsys/dw-hdmi: Enable cec clock

[Hans Verkuil]

On 10/14/2017 12:53 AM, Pierre-Hugues Husson wrote:
> The documentation already mentions "cec" optional clock, but
> currently the driver doesn't enable it.
> 
> Signed-off-by: Pierre-Hugues Husson <phh@phh.me>

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Thanks!

	Hans

> ---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index bf14214fa464..5007cdf43131 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -138,6 +138,7 @@ struct dw_hdmi {
>  	struct device *dev;
>  	struct clk *isfr_clk;
>  	struct clk *iahb_clk;
> +	struct clk *cec_clk;
>  	struct dw_hdmi_i2c *i2c;
>  
>  	struct hdmi_data_info hdmi_data;
> @@ -2382,6 +2383,18 @@ __dw_hdmi_probe(struct platform_device *pdev,
>  		goto err_isfr;
>  	}
>  
> +	hdmi->cec_clk = devm_clk_get(hdmi->dev, "cec");
> +	if (IS_ERR(hdmi->cec_clk)) {
> +		hdmi->cec_clk = NULL;
> +	} else {
> +		ret = clk_prepare_enable(hdmi->cec_clk);
> +		if (ret) {
> +			dev_err(hdmi->dev, "Cannot enable HDMI cec clock: %d\n",
> +					ret);
> +			goto err_res;
> +		}
> +	}
> +
>  	/* Product and revision IDs */
>  	hdmi->version = (hdmi_readb(hdmi, HDMI_DESIGN_ID) << 8)
>  		      | (hdmi_readb(hdmi, HDMI_REVISION_ID) << 0);
> @@ -2518,6 +2531,8 @@ __dw_hdmi_probe(struct platform_device *pdev,
>  		cec_notifier_put(hdmi->cec_notifier);
>  
>  	clk_disable_unprepare(hdmi->iahb_clk);
> +	if (hdmi->cec_clk)
> +		clk_disable_unprepare(hdmi->cec_clk);
>  err_isfr:
>  	clk_disable_unprepare(hdmi->isfr_clk);
>  err_res:
> @@ -2541,6 +2556,8 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi)
>  
>  	clk_disable_unprepare(hdmi->iahb_clk);
>  	clk_disable_unprepare(hdmi->isfr_clk);
> +	if (hdmi->cec_clk)
> +		clk_disable_unprepare(hdmi->cec_clk);
>  
>  	if (hdmi->i2c)
>  		i2c_del_adapter(&hdmi->i2c->adap);
> 

Re: [PATCH 1/3] drm: bridge: synopsys/dw-hdmi: Enable cec clock

[Hans Verkuil]

On 14/10/17 10:16, Hans Verkuil wrote:
> On 10/14/2017 12:53 AM, Pierre-Hugues Husson wrote:
>> The documentation already mentions "cec" optional clock, but
>> currently the driver doesn't enable it.
>>
>> Signed-off-by: Pierre-Hugues Husson <phh@phh.me>
> 
> Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Tested-by: Hans Verkuil <hans.verkuil@cisco.com>

> 
> Thanks!
> 
> 	Hans
> 
>> ---
>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> index bf14214fa464..5007cdf43131 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> @@ -138,6 +138,7 @@ struct dw_hdmi {
>>  	struct device *dev;
>>  	struct clk *isfr_clk;
>>  	struct clk *iahb_clk;
>> +	struct clk *cec_clk;
>>  	struct dw_hdmi_i2c *i2c;
>>  
>>  	struct hdmi_data_info hdmi_data;
>> @@ -2382,6 +2383,18 @@ __dw_hdmi_probe(struct platform_device *pdev,
>>  		goto err_isfr;
>>  	}
>>  
>> +	hdmi->cec_clk = devm_clk_get(hdmi->dev, "cec");
>> +	if (IS_ERR(hdmi->cec_clk)) {
>> +		hdmi->cec_clk = NULL;
>> +	} else {
>> +		ret = clk_prepare_enable(hdmi->cec_clk);
>> +		if (ret) {
>> +			dev_err(hdmi->dev, "Cannot enable HDMI cec clock: %d\n",
>> +					ret);
>> +			goto err_res;
>> +		}
>> +	}
>> +
>>  	/* Product and revision IDs */
>>  	hdmi->version = (hdmi_readb(hdmi, HDMI_DESIGN_ID) << 8)
>>  		      | (hdmi_readb(hdmi, HDMI_REVISION_ID) << 0);
>> @@ -2518,6 +2531,8 @@ __dw_hdmi_probe(struct platform_device *pdev,
>>  		cec_notifier_put(hdmi->cec_notifier);
>>  
>>  	clk_disable_unprepare(hdmi->iahb_clk);
>> +	if (hdmi->cec_clk)
>> +		clk_disable_unprepare(hdmi->cec_clk);
>>  err_isfr:
>>  	clk_disable_unprepare(hdmi->isfr_clk);
>>  err_res:
>> @@ -2541,6 +2556,8 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi)
>>  
>>  	clk_disable_unprepare(hdmi->iahb_clk);
>>  	clk_disable_unprepare(hdmi->isfr_clk);
>> +	if (hdmi->cec_clk)
>> +		clk_disable_unprepare(hdmi->cec_clk);
>>  
>>  	if (hdmi->i2c)
>>  		i2c_del_adapter(&hdmi->i2c->adap);
>>
> 

[PATCH 1/3] drm: bridge: synopsys/dw-hdmi: Enable cec clock

[Hans Verkuil]

On 14/10/17 10:16, Hans Verkuil wrote:
> On 10/14/2017 12:53 AM, Pierre-Hugues Husson wrote:
>> The documentation already mentions "cec" optional clock, but
>> currently the driver doesn't enable it.
>>
>> Signed-off-by: Pierre-Hugues Husson <phh@phh.me>
> 
> Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Tested-by: Hans Verkuil <hans.verkuil@cisco.com>

> 
> Thanks!
> 
> 	Hans
> 
>> ---
>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> index bf14214fa464..5007cdf43131 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> @@ -138,6 +138,7 @@ struct dw_hdmi {
>>  	struct device *dev;
>>  	struct clk *isfr_clk;
>>  	struct clk *iahb_clk;
>> +	struct clk *cec_clk;
>>  	struct dw_hdmi_i2c *i2c;
>>  
>>  	struct hdmi_data_info hdmi_data;
>> @@ -2382,6 +2383,18 @@ __dw_hdmi_probe(struct platform_device *pdev,
>>  		goto err_isfr;
>>  	}
>>  
>> +	hdmi->cec_clk = devm_clk_get(hdmi->dev, "cec");
>> +	if (IS_ERR(hdmi->cec_clk)) {
>> +		hdmi->cec_clk = NULL;
>> +	} else {
>> +		ret = clk_prepare_enable(hdmi->cec_clk);
>> +		if (ret) {
>> +			dev_err(hdmi->dev, "Cannot enable HDMI cec clock: %d\n",
>> +					ret);
>> +			goto err_res;
>> +		}
>> +	}
>> +
>>  	/* Product and revision IDs */
>>  	hdmi->version = (hdmi_readb(hdmi, HDMI_DESIGN_ID) << 8)
>>  		      | (hdmi_readb(hdmi, HDMI_REVISION_ID) << 0);
>> @@ -2518,6 +2531,8 @@ __dw_hdmi_probe(struct platform_device *pdev,
>>  		cec_notifier_put(hdmi->cec_notifier);
>>  
>>  	clk_disable_unprepare(hdmi->iahb_clk);
>> +	if (hdmi->cec_clk)
>> +		clk_disable_unprepare(hdmi->cec_clk);
>>  err_isfr:
>>  	clk_disable_unprepare(hdmi->isfr_clk);
>>  err_res:
>> @@ -2541,6 +2556,8 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi)
>>  
>>  	clk_disable_unprepare(hdmi->iahb_clk);
>>  	clk_disable_unprepare(hdmi->isfr_clk);
>> +	if (hdmi->cec_clk)
>> +		clk_disable_unprepare(hdmi->cec_clk);
>>  
>>  	if (hdmi->i2c)
>>  		i2c_del_adapter(&hdmi->i2c->adap);
>>
> 

Re: [PATCH 1/3] drm: bridge: synopsys/dw-hdmi: Enable cec clock

[Russell King - ARM Linux]

On Sat, Oct 14, 2017 at 12:53:35AM +0200, Pierre-Hugues Husson wrote:
> @@ -2382,6 +2383,18 @@ __dw_hdmi_probe(struct platform_device *pdev,
>  		goto err_isfr;
>  	}
>  
> +	hdmi->cec_clk = devm_clk_get(hdmi->dev, "cec");
> +	if (IS_ERR(hdmi->cec_clk)) {
> +		hdmi->cec_clk = NULL;

What if devm_clk_get() returns EPROBE_DEFER?  Does that really mean the
clock does not exist?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH 1/3] drm: bridge: synopsys/dw-hdmi: Enable cec clock

[Russell King - ARM Linux]

On Sat, Oct 14, 2017 at 12:53:35AM +0200, Pierre-Hugues Husson wrote:
> @@ -2382,6 +2383,18 @@ __dw_hdmi_probe(struct platform_device *pdev,
>  		goto err_isfr;
>  	}
>  
> +	hdmi->cec_clk = devm_clk_get(hdmi->dev, "cec");
> +	if (IS_ERR(hdmi->cec_clk)) {
> +		hdmi->cec_clk = NULL;

What if devm_clk_get() returns EPROBE_DEFER?  Does that really mean the
clock does not exist?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

Re: [PATCH 1/3] drm: bridge: synopsys/dw-hdmi: Enable cec clock

[Pierre-Hugues Husson]

2017-10-20 10:12 GMT+02:00 Russell King - ARM Linux <linux@armlinux.org.uk>:
> On Sat, Oct 14, 2017 at 12:53:35AM +0200, Pierre-Hugues Husson wrote:
>> @@ -2382,6 +2383,18 @@ __dw_hdmi_probe(struct platform_device *pdev,
>>               goto err_isfr;
>>       }
>>
>> +     hdmi->cec_clk = devm_clk_get(hdmi->dev, "cec");
>> +     if (IS_ERR(hdmi->cec_clk)) {
>> +             hdmi->cec_clk = NULL;
>
> What if devm_clk_get() returns EPROBE_DEFER?  Does that really mean the
> clock does not exist?
Should be fixed in v2.
Thanks

[PATCH 1/3] drm: bridge: synopsys/dw-hdmi: Enable cec clock

[Pierre-Hugues Husson]

2017-10-20 10:12 GMT+02:00 Russell King - ARM Linux <linux@armlinux.org.uk>:
> On Sat, Oct 14, 2017 at 12:53:35AM +0200, Pierre-Hugues Husson wrote:
>> @@ -2382,6 +2383,18 @@ __dw_hdmi_probe(struct platform_device *pdev,
>>               goto err_isfr;
>>       }
>>
>> +     hdmi->cec_clk = devm_clk_get(hdmi->dev, "cec");
>> +     if (IS_ERR(hdmi->cec_clk)) {
>> +             hdmi->cec_clk = NULL;
>
> What if devm_clk_get() returns EPROBE_DEFER?  Does that really mean the
> clock does not exist?
Should be fixed in v2.
Thanks

[PATCH 2/3] arm64: dts: rockchip: add the cec clk for dw-mipi-hdmi on rk3399

[Pierre-Hugues Husson]

Signed-off-by: Pierre-Hugues Husson <phh@phh.me>
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index ab7629c5b856..a65f7f744a40 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1601,8 +1601,12 @@
 		compatible = "rockchip,rk3399-dw-hdmi";
 		reg = <0x0 0xff940000 0x0 0x20000>;
 		interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH 0>;
-		clocks = <&cru PCLK_HDMI_CTRL>, <&cru SCLK_HDMI_SFR>, <&cru PLL_VPLL>, <&cru PCLK_VIO_GRF>;
-		clock-names = "iahb", "isfr", "vpll", "grf";
+		clocks = <&cru PCLK_HDMI_CTRL>,
+			 <&cru SCLK_HDMI_SFR>,
+			 <&cru PLL_VPLL>,
+			 <&cru PCLK_VIO_GRF>,
+			 <&cru SCLK_HDMI_CEC>;
+		clock-names = "iahb", "isfr", "vpll", "grf", "cec";
 		power-domains = <&power RK3399_PD_HDCP>;
 		reg-io-width = <4>;
 		rockchip,grf = <&grf>;
-- 
2.14.1

[PATCH 2/3] arm64: dts: rockchip: add the cec clk for dw-mipi-hdmi on rk3399

[Pierre-Hugues Husson]

Signed-off-by: Pierre-Hugues Husson <phh@phh.me>
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index ab7629c5b856..a65f7f744a40 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1601,8 +1601,12 @@
 		compatible = "rockchip,rk3399-dw-hdmi";
 		reg = <0x0 0xff940000 0x0 0x20000>;
 		interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH 0>;
-		clocks = <&cru PCLK_HDMI_CTRL>, <&cru SCLK_HDMI_SFR>, <&cru PLL_VPLL>, <&cru PCLK_VIO_GRF>;
-		clock-names = "iahb", "isfr", "vpll", "grf";
+		clocks = <&cru PCLK_HDMI_CTRL>,
+			 <&cru SCLK_HDMI_SFR>,
+			 <&cru PLL_VPLL>,
+			 <&cru PCLK_VIO_GRF>,
+			 <&cru SCLK_HDMI_CEC>;
+		clock-names = "iahb", "isfr", "vpll", "grf", "cec";
 		power-domains = <&power RK3399_PD_HDCP>;
 		reg-io-width = <4>;
 		rockchip,grf = <&grf>;
-- 
2.14.1

Re: [PATCH 2/3] arm64: dts: rockchip: add the cec clk for dw-mipi-hdmi on rk3399

[Heiko Stuebner]

Am Samstag, 14. Oktober 2017, 00:53:36 CEST schrieb Pierre-Hugues Husson:
> Signed-off-by: Pierre-Hugues Husson <phh@phh.me>

applied for 4.15 after adding a basic commit message.
It is custom to always provide at least some sort of message there.


Thanks
Heiko

[PATCH 2/3] arm64: dts: rockchip: add the cec clk for dw-mipi-hdmi on rk3399

[Heiko Stuebner]

Am Samstag, 14. Oktober 2017, 00:53:36 CEST schrieb Pierre-Hugues Husson:
> Signed-off-by: Pierre-Hugues Husson <phh@phh.me>

applied for 4.15 after adding a basic commit message.
It is custom to always provide at least some sort of message there.


Thanks
Heiko

[PATCH 3/3] arm64: dts: rockchip: enable cec pin for rk3399 firefly

[Pierre-Hugues Husson]

Signed-off-by: Pierre-Hugues Husson <phh@phh.me>
---
 arch/arm64/boot/dts/rockchip/rk3399-firefly.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts b/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts
index f6fbcc05073e..431ff1bb3720 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts
@@ -257,6 +257,8 @@
 
 &hdmi {
 	ddc-i2c-bus = <&i2c3>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&hdmi_cec>;
 	status = "okay";
 };
 
-- 
2.14.1

[PATCH 3/3] arm64: dts: rockchip: enable cec pin for rk3399 firefly

[Pierre-Hugues Husson]

Signed-off-by: Pierre-Hugues Husson <phh@phh.me>
---
 arch/arm64/boot/dts/rockchip/rk3399-firefly.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts b/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts
index f6fbcc05073e..431ff1bb3720 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts
@@ -257,6 +257,8 @@
 
 &hdmi {
 	ddc-i2c-bus = <&i2c3>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&hdmi_cec>;
 	status = "okay";
 };
 
-- 
2.14.1

Re: [PATCH 3/3] arm64: dts: rockchip: enable cec pin for rk3399 firefly

[Heiko Stuebner]

Am Samstag, 14. Oktober 2017, 00:53:37 CEST schrieb Pierre-Hugues Husson:
> Signed-off-by: Pierre-Hugues Husson <phh@phh.me>

applied for 4.15 after adding a basic commit message.
It is custom to always provide at least some sort of message there.


Thanks
Heiko

[PATCH 3/3] arm64: dts: rockchip: enable cec pin for rk3399 firefly

[Heiko Stuebner]

Am Samstag, 14. Oktober 2017, 00:53:37 CEST schrieb Pierre-Hugues Husson:
> Signed-off-by: Pierre-Hugues Husson <phh@phh.me>

applied for 4.15 after adding a basic commit message.
It is custom to always provide at least some sort of message there.


Thanks
Heiko

Re: [PATCH 0/3] Enable CEC on rk3399

[Hans Verkuil]

On 10/14/2017 12:53 AM, Pierre-Hugues Husson wrote:
> Enable CEC on firefly-rk3399.
> Tested on a TV with cec-ctl --playback; cec-ctl -S
> 
> Pierre-Hugues Husson (3):
>   drm: bridge: synopsys/dw-hdmi: Enable cec clock
>   arm64: dts: rockchip: add the cec clk for dw-mipi-hdmi on rk3399
>   arm64: dts: rockchip: enable cec pin for rk3399 firefly
> 
>  arch/arm64/boot/dts/rockchip/rk3399-firefly.dts |  2 ++
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi        |  8 ++++++--
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c       | 16 ++++++++++++++++
>  3 files changed, 24 insertions(+), 2 deletions(-)
> 

Nice! I had a similar dw-hdmi.c patch pending but got around to posting it.

I'll brush off my old rk3288 patches and see if I can get CEC enabled
for my firefly-reload. I was close to getting it work, but I guess
missed the "enable cec pin" change.

Regards,

	Hans

[PATCH 0/3] Enable CEC on rk3399

[Hans Verkuil]

On 10/14/2017 12:53 AM, Pierre-Hugues Husson wrote:
> Enable CEC on firefly-rk3399.
> Tested on a TV with cec-ctl --playback; cec-ctl -S
> 
> Pierre-Hugues Husson (3):
>   drm: bridge: synopsys/dw-hdmi: Enable cec clock
>   arm64: dts: rockchip: add the cec clk for dw-mipi-hdmi on rk3399
>   arm64: dts: rockchip: enable cec pin for rk3399 firefly
> 
>  arch/arm64/boot/dts/rockchip/rk3399-firefly.dts |  2 ++
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi        |  8 ++++++--
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c       | 16 ++++++++++++++++
>  3 files changed, 24 insertions(+), 2 deletions(-)
> 

Nice! I had a similar dw-hdmi.c patch pending but got around to posting it.

I'll brush off my old rk3288 patches and see if I can get CEC enabled
for my firefly-reload. I was close to getting it work, but I guess
missed the "enable cec pin" change.

Regards,

	Hans

Re: [PATCH 0/3] Enable CEC on rk3399

[Pierre-Hugues Husson]

Hi Hans,

> Nice! I had a similar dw-hdmi.c patch pending but got around to posting it.
>
> I'll brush off my old rk3288 patches and see if I can get CEC enabled
> for my firefly-reload. I was close to getting it work, but I guess
> missed the "enable cec pin" change.
Please note that on rk3288, there are two CEC pins, and you must write
in RK3288_GRF_SOC_CON8 which pin you're using.
On the firefly-reload, the pin used is GPIO7C0, while the default pin
configuration is GPIO7C7.

Regards,

[PATCH 0/3] Enable CEC on rk3399

[Pierre-Hugues Husson]

Hi Hans,

> Nice! I had a similar dw-hdmi.c patch pending but got around to posting it.
>
> I'll brush off my old rk3288 patches and see if I can get CEC enabled
> for my firefly-reload. I was close to getting it work, but I guess
> missed the "enable cec pin" change.
Please note that on rk3288, there are two CEC pins, and you must write
in RK3288_GRF_SOC_CON8 which pin you're using.
On the firefly-reload, the pin used is GPIO7C0, while the default pin
configuration is GPIO7C7.

Regards,

Re: [PATCH 0/3] Enable CEC on rk3399

[Heiko Stuebner]

Am Samstag, 14. Oktober 2017, 15:14:40 CEST schrieb Pierre-Hugues Husson:
> Hi Hans,
> 
> > Nice! I had a similar dw-hdmi.c patch pending but got around to posting it.
> >
> > I'll brush off my old rk3288 patches and see if I can get CEC enabled
> > for my firefly-reload. I was close to getting it work, but I guess
> > missed the "enable cec pin" change.
> Please note that on rk3288, there are two CEC pins, and you must write
> in RK3288_GRF_SOC_CON8 which pin you're using.
> On the firefly-reload, the pin used is GPIO7C0, while the default pin
> configuration is GPIO7C7.

And as an additional note, later socs have even more of these pin-routing
settings and we currently have infrastructure in the pinctrl driver to do
this automatically depending on the pinctrl settings.

So most likely this can also be added there for the rk3288.


Heiko

[PATCH 0/3] Enable CEC on rk3399

[Heiko Stuebner]

Am Samstag, 14. Oktober 2017, 15:14:40 CEST schrieb Pierre-Hugues Husson:
> Hi Hans,
> 
> > Nice! I had a similar dw-hdmi.c patch pending but got around to posting it.
> >
> > I'll brush off my old rk3288 patches and see if I can get CEC enabled
> > for my firefly-reload. I was close to getting it work, but I guess
> > missed the "enable cec pin" change.
> Please note that on rk3288, there are two CEC pins, and you must write
> in RK3288_GRF_SOC_CON8 which pin you're using.
> On the firefly-reload, the pin used is GPIO7C0, while the default pin
> configuration is GPIO7C7.

And as an additional note, later socs have even more of these pin-routing
settings and we currently have infrastructure in the pinctrl driver to do
this automatically depending on the pinctrl settings.

So most likely this can also be added there for the rk3288.


Heiko

[PATCH] pinctrl: rockchip: Add iomux-route switching support for rk3288

[Heiko Stuebner]

So far only the hdmi cec supports using one of two different pins
as source, so add the route switching for it.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
If I didn't mess up any numbering, the pinctrl change should look like
the following patch.

Hope that helps
Heiko

 drivers/pinctrl/pinctrl-rockchip.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index b5cb7858ffdc..8dd49e2e144b 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -884,6 +884,24 @@ static struct rockchip_mux_route_data rk3228_mux_route_data[] = {
 	},
 };
 
+static struct rockchip_mux_route_data rk3288_mux_route_data[] = {
+	{
+		/* edphdmi_cecinoutt1 */
+		.bank_num = 7,
+		.pin = 16,
+		.func = 2,
+		.route_offset = 0x264,
+		.route_val = BIT(16 + 12) | BIT(12),
+	}, {
+		/* edphdmi_cecinout */
+		.bank_num = 7,
+		.pin = 23,
+		.func = 4,
+		.route_offset = 0x264,
+		.route_val = BIT(16 + 12),
+	},
+};
+
 static struct rockchip_mux_route_data rk3328_mux_route_data[] = {
 	{
 		/* uart2dbg_rxm0 */
@@ -3391,6 +3409,8 @@ static struct rockchip_pin_ctrl rk3288_pin_ctrl = {
 		.type			= RK3288,
 		.grf_mux_offset		= 0x0,
 		.pmu_mux_offset		= 0x84,
+		.iomux_routes		= rk3288_mux_route_data,
+		.niomux_routes		= ARRAY_SIZE(rk3288_mux_route_data),
 		.pull_calc_reg		= rk3288_calc_pull_reg_and_bit,
 		.drv_calc_reg		= rk3288_calc_drv_reg_and_bit,
 };
-- 
2.14.1

[PATCH] pinctrl: rockchip: Add iomux-route switching support for rk3288

[Heiko Stuebner]

So far only the hdmi cec supports using one of two different pins
as source, so add the route switching for it.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
If I didn't mess up any numbering, the pinctrl change should look like
the following patch.

Hope that helps
Heiko

 drivers/pinctrl/pinctrl-rockchip.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index b5cb7858ffdc..8dd49e2e144b 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -884,6 +884,24 @@ static struct rockchip_mux_route_data rk3228_mux_route_data[] = {
 	},
 };
 
+static struct rockchip_mux_route_data rk3288_mux_route_data[] = {
+	{
+		/* edphdmi_cecinoutt1 */
+		.bank_num = 7,
+		.pin = 16,
+		.func = 2,
+		.route_offset = 0x264,
+		.route_val = BIT(16 + 12) | BIT(12),
+	}, {
+		/* edphdmi_cecinout */
+		.bank_num = 7,
+		.pin = 23,
+		.func = 4,
+		.route_offset = 0x264,
+		.route_val = BIT(16 + 12),
+	},
+};
+
 static struct rockchip_mux_route_data rk3328_mux_route_data[] = {
 	{
 		/* uart2dbg_rxm0 */
@@ -3391,6 +3409,8 @@ static struct rockchip_pin_ctrl rk3288_pin_ctrl = {
 		.type			= RK3288,
 		.grf_mux_offset		= 0x0,
 		.pmu_mux_offset		= 0x84,
+		.iomux_routes		= rk3288_mux_route_data,
+		.niomux_routes		= ARRAY_SIZE(rk3288_mux_route_data),
 		.pull_calc_reg		= rk3288_calc_pull_reg_and_bit,
 		.drv_calc_reg		= rk3288_calc_drv_reg_and_bit,
 };
-- 
2.14.1

Re: [PATCH] pinctrl: rockchip: Add iomux-route switching support for rk3288

[Hans Verkuil]

On 14/10/17 17:39, Heiko Stuebner wrote:
> So far only the hdmi cec supports using one of two different pins
> as source, so add the route switching for it.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>

Just tested this on my firefly reload and it works great!

Tested-by: Hans Verkuil <hans.verkuil@cisco.com>

I'll post some dts patches later today to fully bring up the first HDMI
output on the Firefly Reload.

Will you process this patch further to get it mainlined?

Regards,

	Hans

> ---
> If I didn't mess up any numbering, the pinctrl change should look like
> the following patch.
> 
> Hope that helps
> Heiko
> 
>  drivers/pinctrl/pinctrl-rockchip.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
> index b5cb7858ffdc..8dd49e2e144b 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
> @@ -884,6 +884,24 @@ static struct rockchip_mux_route_data rk3228_mux_route_data[] = {
>  	},
>  };
>  
> +static struct rockchip_mux_route_data rk3288_mux_route_data[] = {
> +	{
> +		/* edphdmi_cecinoutt1 */
> +		.bank_num = 7,
> +		.pin = 16,
> +		.func = 2,
> +		.route_offset = 0x264,
> +		.route_val = BIT(16 + 12) | BIT(12),
> +	}, {
> +		/* edphdmi_cecinout */
> +		.bank_num = 7,
> +		.pin = 23,
> +		.func = 4,
> +		.route_offset = 0x264,
> +		.route_val = BIT(16 + 12),
> +	},
> +};
> +
>  static struct rockchip_mux_route_data rk3328_mux_route_data[] = {
>  	{
>  		/* uart2dbg_rxm0 */
> @@ -3391,6 +3409,8 @@ static struct rockchip_pin_ctrl rk3288_pin_ctrl = {
>  		.type			= RK3288,
>  		.grf_mux_offset		= 0x0,
>  		.pmu_mux_offset		= 0x84,
> +		.iomux_routes		= rk3288_mux_route_data,
> +		.niomux_routes		= ARRAY_SIZE(rk3288_mux_route_data),
>  		.pull_calc_reg		= rk3288_calc_pull_reg_and_bit,
>  		.drv_calc_reg		= rk3288_calc_drv_reg_and_bit,
>  };
> 

[PATCH] pinctrl: rockchip: Add iomux-route switching support for rk3288

[Hans Verkuil]

On 14/10/17 17:39, Heiko Stuebner wrote:
> So far only the hdmi cec supports using one of two different pins
> as source, so add the route switching for it.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>

Just tested this on my firefly reload and it works great!

Tested-by: Hans Verkuil <hans.verkuil@cisco.com>

I'll post some dts patches later today to fully bring up the first HDMI
output on the Firefly Reload.

Will you process this patch further to get it mainlined?

Regards,

	Hans

> ---
> If I didn't mess up any numbering, the pinctrl change should look like
> the following patch.
> 
> Hope that helps
> Heiko
> 
>  drivers/pinctrl/pinctrl-rockchip.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
> index b5cb7858ffdc..8dd49e2e144b 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
> @@ -884,6 +884,24 @@ static struct rockchip_mux_route_data rk3228_mux_route_data[] = {
>  	},
>  };
>  
> +static struct rockchip_mux_route_data rk3288_mux_route_data[] = {
> +	{
> +		/* edphdmi_cecinoutt1 */
> +		.bank_num = 7,
> +		.pin = 16,
> +		.func = 2,
> +		.route_offset = 0x264,
> +		.route_val = BIT(16 + 12) | BIT(12),
> +	}, {
> +		/* edphdmi_cecinout */
> +		.bank_num = 7,
> +		.pin = 23,
> +		.func = 4,
> +		.route_offset = 0x264,
> +		.route_val = BIT(16 + 12),
> +	},
> +};
> +
>  static struct rockchip_mux_route_data rk3328_mux_route_data[] = {
>  	{
>  		/* uart2dbg_rxm0 */
> @@ -3391,6 +3409,8 @@ static struct rockchip_pin_ctrl rk3288_pin_ctrl = {
>  		.type			= RK3288,
>  		.grf_mux_offset		= 0x0,
>  		.pmu_mux_offset		= 0x84,
> +		.iomux_routes		= rk3288_mux_route_data,
> +		.niomux_routes		= ARRAY_SIZE(rk3288_mux_route_data),
>  		.pull_calc_reg		= rk3288_calc_pull_reg_and_bit,
>  		.drv_calc_reg		= rk3288_calc_drv_reg_and_bit,
>  };
> 

Re: [PATCH] pinctrl: rockchip: Add iomux-route switching support for rk3288

[Heiko Stübner]

Hi Hans,

Am Freitag, 20. Oktober 2017, 09:28:58 CEST schrieb Hans Verkuil:
> On 14/10/17 17:39, Heiko Stuebner wrote:
> > So far only the hdmi cec supports using one of two different pins
> > as source, so add the route switching for it.
> > 
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> 
> Just tested this on my firefly reload and it works great!
> 
> Tested-by: Hans Verkuil <hans.verkuil@cisco.com>

oh cool. I really only wrote this based on the soc manual,
so it actually surprises me, that it works on the first try :-)

> I'll post some dts patches later today to fully bring up the first HDMI
> output on the Firefly Reload.
> 
> Will you process this patch further to get it mainlined?

Yep, I'll do that.


Heiko

[PATCH] pinctrl: rockchip: Add iomux-route switching support for rk3288

[Heiko Stübner]

Hi Hans,

Am Freitag, 20. Oktober 2017, 09:28:58 CEST schrieb Hans Verkuil:
> On 14/10/17 17:39, Heiko Stuebner wrote:
> > So far only the hdmi cec supports using one of two different pins
> > as source, so add the route switching for it.
> > 
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> 
> Just tested this on my firefly reload and it works great!
> 
> Tested-by: Hans Verkuil <hans.verkuil@cisco.com>

oh cool. I really only wrote this based on the soc manual,
so it actually surprises me, that it works on the first try :-)

> I'll post some dts patches later today to fully bring up the first HDMI
> output on the Firefly Reload.
> 
> Will you process this patch further to get it mainlined?

Yep, I'll do that.


Heiko

Re: [PATCH] pinctrl: rockchip: Add iomux-route switching support for rk3288

[Hans Verkuil]

On 20/10/17 09:38, Heiko Stübner wrote:
> Hi Hans,
> 
> Am Freitag, 20. Oktober 2017, 09:28:58 CEST schrieb Hans Verkuil:
>> On 14/10/17 17:39, Heiko Stuebner wrote:
>>> So far only the hdmi cec supports using one of two different pins
>>> as source, so add the route switching for it.
>>>
>>> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
>>
>> Just tested this on my firefly reload and it works great!
>>
>> Tested-by: Hans Verkuil <hans.verkuil@cisco.com>
> 
> oh cool. I really only wrote this based on the soc manual,
> so it actually surprises me, that it works on the first try :-)

One note though: I've only tested it on my Firefly Reload. I don't have a
regular Firefly, so I can't be certain it works there. Just covering my ass
here :-)

> 
>> I'll post some dts patches later today to fully bring up the first HDMI
>> output on the Firefly Reload.
>>
>> Will you process this patch further to get it mainlined?
> 
> Yep, I'll do that.

Thanks!

Regards,

	Hans

[PATCH] pinctrl: rockchip: Add iomux-route switching support for rk3288

[Hans Verkuil]

On 20/10/17 09:38, Heiko St?bner wrote:
> Hi Hans,
> 
> Am Freitag, 20. Oktober 2017, 09:28:58 CEST schrieb Hans Verkuil:
>> On 14/10/17 17:39, Heiko Stuebner wrote:
>>> So far only the hdmi cec supports using one of two different pins
>>> as source, so add the route switching for it.
>>>
>>> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
>>
>> Just tested this on my firefly reload and it works great!
>>
>> Tested-by: Hans Verkuil <hans.verkuil@cisco.com>
> 
> oh cool. I really only wrote this based on the soc manual,
> so it actually surprises me, that it works on the first try :-)

One note though: I've only tested it on my Firefly Reload. I don't have a
regular Firefly, so I can't be certain it works there. Just covering my ass
here :-)

> 
>> I'll post some dts patches later today to fully bring up the first HDMI
>> output on the Firefly Reload.
>>
>> Will you process this patch further to get it mainlined?
> 
> Yep, I'll do that.

Thanks!

Regards,

	Hans

Re: [PATCH] pinctrl: rockchip: Add iomux-route switching support for rk3288

[Heiko Stübner]

Am Freitag, 20. Oktober 2017, 09:44:55 CEST schrieb Hans Verkuil:
> On 20/10/17 09:38, Heiko Stübner wrote:
> > Hi Hans,
> > 
> > Am Freitag, 20. Oktober 2017, 09:28:58 CEST schrieb Hans Verkuil:
> >> On 14/10/17 17:39, Heiko Stuebner wrote:
> >>> So far only the hdmi cec supports using one of two different pins
> >>> as source, so add the route switching for it.
> >>> 
> >>> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> >> 
> >> Just tested this on my firefly reload and it works great!
> >> 
> >> Tested-by: Hans Verkuil <hans.verkuil@cisco.com>
> > 
> > oh cool. I really only wrote this based on the soc manual,
> > so it actually surprises me, that it works on the first try :-)
> 
> One note though: I've only tested it on my Firefly Reload. I don't have a
> regular Firefly, so I can't be certain it works there. Just covering my ass
> here :-)

Haha ... I guess the only thing I could have messed up would be the
ordering (valu0 -> gpio0, value1 -> gpio7 ... and reverse), so if it were
really wrong, you shouldn've have seen any results at all.

[PATCH] pinctrl: rockchip: Add iomux-route switching support for rk3288

[Heiko Stübner]

Am Freitag, 20. Oktober 2017, 09:44:55 CEST schrieb Hans Verkuil:
> On 20/10/17 09:38, Heiko St?bner wrote:
> > Hi Hans,
> > 
> > Am Freitag, 20. Oktober 2017, 09:28:58 CEST schrieb Hans Verkuil:
> >> On 14/10/17 17:39, Heiko Stuebner wrote:
> >>> So far only the hdmi cec supports using one of two different pins
> >>> as source, so add the route switching for it.
> >>> 
> >>> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> >> 
> >> Just tested this on my firefly reload and it works great!
> >> 
> >> Tested-by: Hans Verkuil <hans.verkuil@cisco.com>
> > 
> > oh cool. I really only wrote this based on the soc manual,
> > so it actually surprises me, that it works on the first try :-)
> 
> One note though: I've only tested it on my Firefly Reload. I don't have a
> regular Firefly, so I can't be certain it works there. Just covering my ass
> here :-)

Haha ... I guess the only thing I could have messed up would be the
ordering (valu0 -> gpio0, value1 -> gpio7 ... and reverse), so if it were
really wrong, you shouldn've have seen any results at all.

Re: [PATCH] pinctrl: rockchip: Add iomux-route switching support for rk3288

[Heiko Stübner]

Am Freitag, 20. Oktober 2017, 09:44:55 CEST schrieb Hans Verkuil:
> On 20/10/17 09:38, Heiko Stübner wrote:
> > Hi Hans,
> > 
> > Am Freitag, 20. Oktober 2017, 09:28:58 CEST schrieb Hans Verkuil:
> >> On 14/10/17 17:39, Heiko Stuebner wrote:
> >>> So far only the hdmi cec supports using one of two different pins
> >>> as source, so add the route switching for it.
> >>> 
> >>> Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
> >> 
> >> Just tested this on my firefly reload and it works great!
> >> 
> >> Tested-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
> > 
> > oh cool. I really only wrote this based on the soc manual,
> > so it actually surprises me, that it works on the first try :-)
> 
> One note though: I've only tested it on my Firefly Reload. I don't have a
> regular Firefly, so I can't be certain it works there. Just covering my ass
> here :-)

Haha ... I guess the only thing I could have messed up would be the
ordering (valu0 -> gpio0, value1 -> gpio7 ... and reverse), so if it were
really wrong, you shouldn've have seen any results at all.

Re: [PATCH 0/3] Enable CEC on rk3399

[Hans Verkuil]

On 10/14/2017 04:52 PM, Heiko Stuebner wrote:
> Am Samstag, 14. Oktober 2017, 15:14:40 CEST schrieb Pierre-Hugues Husson:
>> Hi Hans,
>>
>>> Nice! I had a similar dw-hdmi.c patch pending but got around to posting it.
>>>
>>> I'll brush off my old rk3288 patches and see if I can get CEC enabled
>>> for my firefly-reload. I was close to getting it work, but I guess
>>> missed the "enable cec pin" change.
>> Please note that on rk3288, there are two CEC pins, and you must write
>> in RK3288_GRF_SOC_CON8 which pin you're using.
>> On the firefly-reload, the pin used is GPIO7C0, while the default pin
>> configuration is GPIO7C7.
> 
> And as an additional note, later socs have even more of these pin-routing
> settings and we currently have infrastructure in the pinctrl driver to do
> this automatically depending on the pinctrl settings.
> 
> So most likely this can also be added there for the rk3288.
> 
> 
> Heiko
> 

How does 'GPIO7C0' translate to a 'rockchip,pins' line?

I have this in rk3288-firefly-reload.dts:

       hdmi {
                hdmi_cec: hdmi-cec {
                        rockchip,pins = <7 16 RK_FUNC_2 &pcfg_pull_none>;
                };
        };

I think this is correct. You can find my patches for the firefly reload here:

https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=firefly2

I don't have access to my firefly reload until Friday, so I can't test this
until then.

I'd be very grateful though if you can check the last three patches in that
branch. It's just a test branch, so the subject/changelog of those patches
are incomplete.

The very last patch adds CEC support for the firefly/firefly beta. I don't
have that hardware, so I won't be able to test that.

Regards,

	Hans

[PATCH 0/3] Enable CEC on rk3399

[Hans Verkuil]

On 10/14/2017 04:52 PM, Heiko Stuebner wrote:
> Am Samstag, 14. Oktober 2017, 15:14:40 CEST schrieb Pierre-Hugues Husson:
>> Hi Hans,
>>
>>> Nice! I had a similar dw-hdmi.c patch pending but got around to posting it.
>>>
>>> I'll brush off my old rk3288 patches and see if I can get CEC enabled
>>> for my firefly-reload. I was close to getting it work, but I guess
>>> missed the "enable cec pin" change.
>> Please note that on rk3288, there are two CEC pins, and you must write
>> in RK3288_GRF_SOC_CON8 which pin you're using.
>> On the firefly-reload, the pin used is GPIO7C0, while the default pin
>> configuration is GPIO7C7.
> 
> And as an additional note, later socs have even more of these pin-routing
> settings and we currently have infrastructure in the pinctrl driver to do
> this automatically depending on the pinctrl settings.
> 
> So most likely this can also be added there for the rk3288.
> 
> 
> Heiko
> 

How does 'GPIO7C0' translate to a 'rockchip,pins' line?

I have this in rk3288-firefly-reload.dts:

       hdmi {
                hdmi_cec: hdmi-cec {
                        rockchip,pins = <7 16 RK_FUNC_2 &pcfg_pull_none>;
                };
        };

I think this is correct. You can find my patches for the firefly reload here:

https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=firefly2

I don't have access to my firefly reload until Friday, so I can't test this
until then.

I'd be very grateful though if you can check the last three patches in that
branch. It's just a test branch, so the subject/changelog of those patches
are incomplete.

The very last patch adds CEC support for the firefly/firefly beta. I don't
have that hardware, so I won't be able to test that.

Regards,

	Hans

Re: [PATCH 0/3] Enable CEC on rk3399

[Heiko Stuebner]

Hi Hans,

Am Sonntag, 15. Oktober 2017, 12:31:29 CEST schrieb Hans Verkuil:
> On 10/14/2017 04:52 PM, Heiko Stuebner wrote:
> > Am Samstag, 14. Oktober 2017, 15:14:40 CEST schrieb Pierre-Hugues Husson:
> >>> Nice! I had a similar dw-hdmi.c patch pending but got around to posting it.
> >>>
> >>> I'll brush off my old rk3288 patches and see if I can get CEC enabled
> >>> for my firefly-reload. I was close to getting it work, but I guess
> >>> missed the "enable cec pin" change.
> >> Please note that on rk3288, there are two CEC pins, and you must write
> >> in RK3288_GRF_SOC_CON8 which pin you're using.
> >> On the firefly-reload, the pin used is GPIO7C0, while the default pin
> >> configuration is GPIO7C7.
> > 
> > And as an additional note, later socs have even more of these pin-routing
> > settings and we currently have infrastructure in the pinctrl driver to do
> > this automatically depending on the pinctrl settings.
> > 
> > So most likely this can also be added there for the rk3288.
> > 
> > 
> > Heiko
> > 
> 
> How does 'GPIO7C0' translate to a 'rockchip,pins' line?
> 
> I have this in rk3288-firefly-reload.dts:
> 
>        hdmi {
>                 hdmi_cec: hdmi-cec {
>                         rockchip,pins = <7 16 RK_FUNC_2 &pcfg_pull_none>;
>                 };
>         };
> 
> I think this is correct. You can find my patches for the firefly reload here:

Yep, looks correct (i.e. we have 8 pins per A,B,C,D bank, so C0 is pin 16)
but as a helping measure we now also have constants, RK_PC7 in your case.

> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=firefly2
> 
> I don't have access to my firefly reload until Friday, so I can't test this
> until then.
> 
> I'd be very grateful though if you can check the last three patches in that
> branch. It's just a test branch, so the subject/changelog of those patches
> are incomplete.

While basically ok, I'd suggest having the cec pin definitions in rk3288.dtsi,
like adding entries "hdmi-cec-c0" and "hdmi-cec-c7" or similar and selecting
the correct one per board, so that we don't need to duplicate them into each
and every board dts.

The cec pin function is after all a function of the soc itself.


> The very last patch adds CEC support for the firefly/firefly beta. I don't
> have that hardware, so I won't be able to test that.

at least in the schematics for the old firefly-rk3288 it specifies gpio7c0
as cec pin, similar to the reload but in your patch you selected gpio7c7?

Especially as gpio7c7 is part of the debug uart, this would be quite strange.


Heiko

[PATCH 0/3] Enable CEC on rk3399

[Heiko Stuebner]

Hi Hans,

Am Sonntag, 15. Oktober 2017, 12:31:29 CEST schrieb Hans Verkuil:
> On 10/14/2017 04:52 PM, Heiko Stuebner wrote:
> > Am Samstag, 14. Oktober 2017, 15:14:40 CEST schrieb Pierre-Hugues Husson:
> >>> Nice! I had a similar dw-hdmi.c patch pending but got around to posting it.
> >>>
> >>> I'll brush off my old rk3288 patches and see if I can get CEC enabled
> >>> for my firefly-reload. I was close to getting it work, but I guess
> >>> missed the "enable cec pin" change.
> >> Please note that on rk3288, there are two CEC pins, and you must write
> >> in RK3288_GRF_SOC_CON8 which pin you're using.
> >> On the firefly-reload, the pin used is GPIO7C0, while the default pin
> >> configuration is GPIO7C7.
> > 
> > And as an additional note, later socs have even more of these pin-routing
> > settings and we currently have infrastructure in the pinctrl driver to do
> > this automatically depending on the pinctrl settings.
> > 
> > So most likely this can also be added there for the rk3288.
> > 
> > 
> > Heiko
> > 
> 
> How does 'GPIO7C0' translate to a 'rockchip,pins' line?
> 
> I have this in rk3288-firefly-reload.dts:
> 
>        hdmi {
>                 hdmi_cec: hdmi-cec {
>                         rockchip,pins = <7 16 RK_FUNC_2 &pcfg_pull_none>;
>                 };
>         };
> 
> I think this is correct. You can find my patches for the firefly reload here:

Yep, looks correct (i.e. we have 8 pins per A,B,C,D bank, so C0 is pin 16)
but as a helping measure we now also have constants, RK_PC7 in your case.

> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=firefly2
> 
> I don't have access to my firefly reload until Friday, so I can't test this
> until then.
> 
> I'd be very grateful though if you can check the last three patches in that
> branch. It's just a test branch, so the subject/changelog of those patches
> are incomplete.

While basically ok, I'd suggest having the cec pin definitions in rk3288.dtsi,
like adding entries "hdmi-cec-c0" and "hdmi-cec-c7" or similar and selecting
the correct one per board, so that we don't need to duplicate them into each
and every board dts.

The cec pin function is after all a function of the soc itself.


> The very last patch adds CEC support for the firefly/firefly beta. I don't
> have that hardware, so I won't be able to test that.

at least in the schematics for the old firefly-rk3288 it specifies gpio7c0
as cec pin, similar to the reload but in your patch you selected gpio7c7?

Especially as gpio7c7 is part of the debug uart, this would be quite strange.


Heiko

Re: [PATCH 0/3] Enable CEC on rk3399

[Heiko Stuebner]

Hi Hans,

Am Sonntag, 15. Oktober 2017, 12:31:29 CEST schrieb Hans Verkuil:
> On 10/14/2017 04:52 PM, Heiko Stuebner wrote:
> > Am Samstag, 14. Oktober 2017, 15:14:40 CEST schrieb Pierre-Hugues Husson:
> >>> Nice! I had a similar dw-hdmi.c patch pending but got around to posting it.
> >>>
> >>> I'll brush off my old rk3288 patches and see if I can get CEC enabled
> >>> for my firefly-reload. I was close to getting it work, but I guess
> >>> missed the "enable cec pin" change.
> >> Please note that on rk3288, there are two CEC pins, and you must write
> >> in RK3288_GRF_SOC_CON8 which pin you're using.
> >> On the firefly-reload, the pin used is GPIO7C0, while the default pin
> >> configuration is GPIO7C7.
> > 
> > And as an additional note, later socs have even more of these pin-routing
> > settings and we currently have infrastructure in the pinctrl driver to do
> > this automatically depending on the pinctrl settings.
> > 
> > So most likely this can also be added there for the rk3288.
> > 
> > 
> > Heiko
> > 
> 
> How does 'GPIO7C0' translate to a 'rockchip,pins' line?
> 
> I have this in rk3288-firefly-reload.dts:
> 
>        hdmi {
>                 hdmi_cec: hdmi-cec {
>                         rockchip,pins = <7 16 RK_FUNC_2 &pcfg_pull_none>;
>                 };
>         };
> 
> I think this is correct. You can find my patches for the firefly reload here:

Yep, looks correct (i.e. we have 8 pins per A,B,C,D bank, so C0 is pin 16)
but as a helping measure we now also have constants, RK_PC7 in your case.

> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=firefly2
> 
> I don't have access to my firefly reload until Friday, so I can't test this
> until then.
> 
> I'd be very grateful though if you can check the last three patches in that
> branch. It's just a test branch, so the subject/changelog of those patches
> are incomplete.

While basically ok, I'd suggest having the cec pin definitions in rk3288.dtsi,
like adding entries "hdmi-cec-c0" and "hdmi-cec-c7" or similar and selecting
the correct one per board, so that we don't need to duplicate them into each
and every board dts.

The cec pin function is after all a function of the soc itself.


> The very last patch adds CEC support for the firefly/firefly beta. I don't
> have that hardware, so I won't be able to test that.

at least in the schematics for the old firefly-rk3288 it specifies gpio7c0
as cec pin, similar to the reload but in your patch you selected gpio7c7?

Especially as gpio7c7 is part of the debug uart, this would be quite strange.


Heiko

[PATCH] pinctrl: rockchip: Add iomux-route switching support for rk3288

[Heiko Stuebner]

The rk3288 also has one function that can be routed to one of two pins,
the hdmi cec functionality can use either gpio7c0 or gpio7c7.
So add the route switching support for it.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/pinctrl/pinctrl-rockchip.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index b5cb7858ffdc..8dd49e2e144b 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -884,6 +884,24 @@ static struct rockchip_mux_route_data rk3228_mux_route_data[] = {
 	},
 };
 
+static struct rockchip_mux_route_data rk3288_mux_route_data[] = {
+	{
+		/* edphdmi_cecinoutt1 */
+		.bank_num = 7,
+		.pin = 16,
+		.func = 2,
+		.route_offset = 0x264,
+		.route_val = BIT(16 + 12) | BIT(12),
+	}, {
+		/* edphdmi_cecinout */
+		.bank_num = 7,
+		.pin = 23,
+		.func = 4,
+		.route_offset = 0x264,
+		.route_val = BIT(16 + 12),
+	},
+};
+
 static struct rockchip_mux_route_data rk3328_mux_route_data[] = {
 	{
 		/* uart2dbg_rxm0 */
@@ -3391,6 +3409,8 @@ static struct rockchip_pin_ctrl rk3288_pin_ctrl = {
 		.type			= RK3288,
 		.grf_mux_offset		= 0x0,
 		.pmu_mux_offset		= 0x84,
+		.iomux_routes		= rk3288_mux_route_data,
+		.niomux_routes		= ARRAY_SIZE(rk3288_mux_route_data),
 		.pull_calc_reg		= rk3288_calc_pull_reg_and_bit,
 		.drv_calc_reg		= rk3288_calc_drv_reg_and_bit,
 };
-- 
2.14.2

Re: [PATCH] pinctrl: rockchip: Add iomux-route switching support for rk3288

[Linus Walleij]

On Sat, Oct 21, 2017 at 10:53 AM, Heiko Stuebner <heiko@sntech.de> wrote:

> The rk3288 also has one function that can be routed to one of two pins,
> the hdmi cec functionality can use either gpio7c0 or gpio7c7.
> So add the route switching support for it.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> Tested-by: Hans Verkuil <hans.verkuil@cisco.com>

Patch applied with David's review tag.

Yours,
Linus Walleij