Re: [PATCH v4.1 1/2] drm/rockchip: analogix_dp: introduce the pclk for grf

[Yakir Yang]

Doug,

On 06/23/2016 01:16 PM, Doug Anderson wrote:

Yakir,

On Wed, Jun 22, 2016 at 6:58 PM, Yakir Yang  wrote:

For RK3399's GRF module, if we want to operate the graphic related grf
registers, we need to enable the pclk_vio_grf which supply power for VIO
GRF IOs, so it's better to introduce an optional grf clock in driver.

Signed-off-by: Yakir Yang 
---
Hi all,

This is an external patch for analogix_dp misc cleanup thread [0]
[0]: https://patchwork.kernel.org/patch/9175613/

BR,
- Yakir

Changes in v4.1:
- Fix compiled error, sorry.
   "dp->cgfclk"  -->  'dp->grfclk'

Changes in v4:
- Check the the error code properly, 'EPROBE_DEFER' should be returned,
   'ENOENT' should assign a NULL point to grfclk, other errors should be
   regarded as failed. (Tomasz, Doug, reviewed at Google Gerrit)
 
[https://chromium-review.googlesource.com/#/c/351821/20/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c@249]
- Add the document about optional 'grf' clock (Tomasz, Doug, reviewed at Google 
Gerrit)
 [https://chromium-review.googlesource.com/#/c/351821/]

Changes in v3:
- Add this patch in v3

  .../display/rockchip/analogix_dp-rockchip.txt  |  6 ++
  drivers/gpu/drm/rockchip/analogix_dp-rockchip.c| 23 +++---
  2 files changed, 26 insertions(+), 3 deletions(-)

I probably would have split into two patches so the bindings was its
own patch, but I don't think it's strictly required.

In any case, this seems good to me.

Reviewed-by: Douglas Anderson 

This is not big change, so collect them into one patch should be more 
cleaner, thanks for your reviewed ;)

Re: [PATCH v4.1 1/2] drm/rockchip: analogix_dp: introduce the pclk for grf

[Yakir Yang]

Doug,

On 06/23/2016 01:16 PM, Doug Anderson wrote:

Yakir,

On Wed, Jun 22, 2016 at 6:58 PM, Yakir Yang  wrote:

For RK3399's GRF module, if we want to operate the graphic related grf
registers, we need to enable the pclk_vio_grf which supply power for VIO
GRF IOs, so it's better to introduce an optional grf clock in driver.

Signed-off-by: Yakir Yang 
---
Hi all,

This is an external patch for analogix_dp misc cleanup thread [0]
[0]: https://patchwork.kernel.org/patch/9175613/

BR,
- Yakir

Changes in v4.1:
- Fix compiled error, sorry.
   "dp->cgfclk"  -->  'dp->grfclk'

Changes in v4:
- Check the the error code properly, 'EPROBE_DEFER' should be returned,
   'ENOENT' should assign a NULL point to grfclk, other errors should be
   regarded as failed. (Tomasz, Doug, reviewed at Google Gerrit)
 
[https://chromium-review.googlesource.com/#/c/351821/20/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c@249]
- Add the document about optional 'grf' clock (Tomasz, Doug, reviewed at Google 
Gerrit)
 [https://chromium-review.googlesource.com/#/c/351821/]

Changes in v3:
- Add this patch in v3

  .../display/rockchip/analogix_dp-rockchip.txt  |  6 ++
  drivers/gpu/drm/rockchip/analogix_dp-rockchip.c| 23 +++---
  2 files changed, 26 insertions(+), 3 deletions(-)

I probably would have split into two patches so the bindings was its
own patch, but I don't think it's strictly required.

In any case, this seems good to me.

Reviewed-by: Douglas Anderson 

This is not big change, so collect them into one patch should be more 
cleaner, thanks for your reviewed ;)

Re: [PATCH v4.1 1/2] drm/rockchip: analogix_dp: introduce the pclk for grf

[Doug Anderson]

Yakir,

On Wed, Jun 22, 2016 at 6:58 PM, Yakir Yang  wrote:
> For RK3399's GRF module, if we want to operate the graphic related grf
> registers, we need to enable the pclk_vio_grf which supply power for VIO
> GRF IOs, so it's better to introduce an optional grf clock in driver.
>
> Signed-off-by: Yakir Yang 
> ---
> Hi all,
>
> This is an external patch for analogix_dp misc cleanup thread [0]
> [0]: https://patchwork.kernel.org/patch/9175613/
>
> BR,
> - Yakir
>
> Changes in v4.1:
> - Fix compiled error, sorry.
>   "dp->cgfclk"  -->  'dp->grfclk'
>
> Changes in v4:
> - Check the the error code properly, 'EPROBE_DEFER' should be returned,
>   'ENOENT' should assign a NULL point to grfclk, other errors should be
>   regarded as failed. (Tomasz, Doug, reviewed at Google Gerrit)
> 
> [https://chromium-review.googlesource.com/#/c/351821/20/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c@249]
> - Add the document about optional 'grf' clock (Tomasz, Doug, reviewed at 
> Google Gerrit)
> [https://chromium-review.googlesource.com/#/c/351821/]
>
> Changes in v3:
> - Add this patch in v3
>
>  .../display/rockchip/analogix_dp-rockchip.txt  |  6 ++
>  drivers/gpu/drm/rockchip/analogix_dp-rockchip.c| 23 
> +++---
>  2 files changed, 26 insertions(+), 3 deletions(-)

I probably would have split into two patches so the bindings was its
own patch, but I don't think it's strictly required.

In any case, this seems good to me.

Reviewed-by: Douglas Anderson 

Re: [PATCH v4.1 1/2] drm/rockchip: analogix_dp: introduce the pclk for grf

[Doug Anderson]

Yakir,

On Wed, Jun 22, 2016 at 6:58 PM, Yakir Yang  wrote:
> For RK3399's GRF module, if we want to operate the graphic related grf
> registers, we need to enable the pclk_vio_grf which supply power for VIO
> GRF IOs, so it's better to introduce an optional grf clock in driver.
>
> Signed-off-by: Yakir Yang 
> ---
> Hi all,
>
> This is an external patch for analogix_dp misc cleanup thread [0]
> [0]: https://patchwork.kernel.org/patch/9175613/
>
> BR,
> - Yakir
>
> Changes in v4.1:
> - Fix compiled error, sorry.
>   "dp->cgfclk"  -->  'dp->grfclk'
>
> Changes in v4:
> - Check the the error code properly, 'EPROBE_DEFER' should be returned,
>   'ENOENT' should assign a NULL point to grfclk, other errors should be
>   regarded as failed. (Tomasz, Doug, reviewed at Google Gerrit)
> 
> [https://chromium-review.googlesource.com/#/c/351821/20/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c@249]
> - Add the document about optional 'grf' clock (Tomasz, Doug, reviewed at 
> Google Gerrit)
> [https://chromium-review.googlesource.com/#/c/351821/]
>
> Changes in v3:
> - Add this patch in v3
>
>  .../display/rockchip/analogix_dp-rockchip.txt  |  6 ++
>  drivers/gpu/drm/rockchip/analogix_dp-rockchip.c| 23 
> +++---
>  2 files changed, 26 insertions(+), 3 deletions(-)

I probably would have split into two patches so the bindings was its
own patch, but I don't think it's strictly required.

In any case, this seems good to me.

Reviewed-by: Douglas Anderson 

[PATCH v4.1 1/2] drm/rockchip: analogix_dp: introduce the pclk for grf

[Yakir Yang]

For RK3399's GRF module, if we want to operate the graphic related grf
registers, we need to enable the pclk_vio_grf which supply power for VIO
GRF IOs, so it's better to introduce an optional grf clock in driver.

Signed-off-by: Yakir Yang 
---
Hi all,

This is an external patch for analogix_dp misc cleanup thread [0]
[0]: https://patchwork.kernel.org/patch/9175613/

BR,
- Yakir

Changes in v4.1:
- Fix compiled error, sorry.
  "dp->cgfclk"  -->  'dp->grfclk'

Changes in v4:
- Check the the error code properly, 'EPROBE_DEFER' should be returned,
  'ENOENT' should assign a NULL point to grfclk, other errors should be
  regarded as failed. (Tomasz, Doug, reviewed at Google Gerrit)

[https://chromium-review.googlesource.com/#/c/351821/20/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c@249]
- Add the document about optional 'grf' clock (Tomasz, Doug, reviewed at Google 
Gerrit)
[https://chromium-review.googlesource.com/#/c/351821/]

Changes in v3:
- Add this patch in v3

 .../display/rockchip/analogix_dp-rockchip.txt  |  6 ++
 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c| 23 +++---
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git 
a/Documentation/devicetree/bindings/display/rockchip/analogix_dp-rockchip.txt 
b/Documentation/devicetree/bindings/display/rockchip/analogix_dp-rockchip.txt
index 726c945..0b39256 100644
--- 
a/Documentation/devicetree/bindings/display/rockchip/analogix_dp-rockchip.txt
+++ 
b/Documentation/devicetree/bindings/display/rockchip/analogix_dp-rockchip.txt
@@ -28,6 +28,12 @@ Required properties:
 Port 0: contained 2 endpoints, connecting to the output of vop.
 Port 1: contained 1 endpoint, connecting to the input of panel.
 
+Optional property for different chips:
+- clocks: from common clock binding: handle to grf_vio clock.
+
+- clock-names: from common clock binding:
+  Required elements: "grf"
+
 For the below properties, please refer to Analogix DP binding document:
  * Documentation/devicetree/bindings/drm/bridge/analogix_dp.txt
 - phys (required)
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c 
b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 787dc51..864ef92 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -56,6 +56,7 @@ struct rockchip_dp_device {
struct drm_display_mode  mode;
 
struct clk   *pclk;
+   struct clk   *grfclk;
struct regmap*grf;
struct reset_control *rst;
 
@@ -151,11 +152,17 @@ static void rockchip_dp_drm_encoder_enable(struct 
drm_encoder *encoder)
 
dev_dbg(dp->dev, "vop %s output to dp\n", (ret) ? "LIT" : "BIG");
 
-   ret = regmap_write(dp->grf, dp->data->lcdsel_grf_reg, val);
-   if (ret != 0) {
-   dev_err(dp->dev, "Could not write to GRF: %d\n", ret);
+   ret = clk_prepare_enable(dp->grfclk);
+   if (ret < 0) {
+   dev_err(dp->dev, "failed to enable grfclk %d\n", ret);
return;
}
+
+   ret = regmap_write(dp->grf, dp->data->lcdsel_grf_reg, val);
+   if (ret != 0)
+   dev_err(dp->dev, "Could not write to GRF: %d\n", ret);
+
+   clk_disable_unprepare(dp->grfclk);
 }
 
 static void rockchip_dp_drm_encoder_nop(struct drm_encoder *encoder)
@@ -235,6 +242,16 @@ static int rockchip_dp_init(struct rockchip_dp_device *dp)
return PTR_ERR(dp->grf);
}
 
+   dp->grfclk = devm_clk_get(dev, "grf");
+   if (PTR_ERR(dp->grfclk) == -ENOENT) {
+   dp->grfclk = NULL;
+   } else if (PTR_ERR(dp->grfclk) == -EPROBE_DEFER) {
+   return -EPROBE_DEFER;
+   } else if (IS_ERR(dp->grfclk)) {
+   dev_err(dev, "failed to get grf clock\n");
+   return PTR_ERR(dp->grfclk);
+   }
+
dp->pclk = devm_clk_get(dev, "pclk");
if (IS_ERR(dp->pclk)) {
dev_err(dev, "failed to get pclk property\n");
-- 
1.9.1

[PATCH v4.1 1/2] drm/rockchip: analogix_dp: introduce the pclk for grf

[Yakir Yang]

For RK3399's GRF module, if we want to operate the graphic related grf
registers, we need to enable the pclk_vio_grf which supply power for VIO
GRF IOs, so it's better to introduce an optional grf clock in driver.

Signed-off-by: Yakir Yang 
---
Hi all,

This is an external patch for analogix_dp misc cleanup thread [0]
[0]: https://patchwork.kernel.org/patch/9175613/

BR,
- Yakir

Changes in v4.1:
- Fix compiled error, sorry.
  "dp->cgfclk"  -->  'dp->grfclk'

Changes in v4:
- Check the the error code properly, 'EPROBE_DEFER' should be returned,
  'ENOENT' should assign a NULL point to grfclk, other errors should be
  regarded as failed. (Tomasz, Doug, reviewed at Google Gerrit)

[https://chromium-review.googlesource.com/#/c/351821/20/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c@249]
- Add the document about optional 'grf' clock (Tomasz, Doug, reviewed at Google 
Gerrit)
[https://chromium-review.googlesource.com/#/c/351821/]

Changes in v3:
- Add this patch in v3

 .../display/rockchip/analogix_dp-rockchip.txt  |  6 ++
 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c| 23 +++---
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git 
a/Documentation/devicetree/bindings/display/rockchip/analogix_dp-rockchip.txt 
b/Documentation/devicetree/bindings/display/rockchip/analogix_dp-rockchip.txt
index 726c945..0b39256 100644
--- 
a/Documentation/devicetree/bindings/display/rockchip/analogix_dp-rockchip.txt
+++ 
b/Documentation/devicetree/bindings/display/rockchip/analogix_dp-rockchip.txt
@@ -28,6 +28,12 @@ Required properties:
 Port 0: contained 2 endpoints, connecting to the output of vop.
 Port 1: contained 1 endpoint, connecting to the input of panel.
 
+Optional property for different chips:
+- clocks: from common clock binding: handle to grf_vio clock.
+
+- clock-names: from common clock binding:
+  Required elements: "grf"
+
 For the below properties, please refer to Analogix DP binding document:
  * Documentation/devicetree/bindings/drm/bridge/analogix_dp.txt
 - phys (required)
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c 
b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 787dc51..864ef92 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -56,6 +56,7 @@ struct rockchip_dp_device {
struct drm_display_mode  mode;
 
struct clk   *pclk;
+   struct clk   *grfclk;
struct regmap*grf;
struct reset_control *rst;
 
@@ -151,11 +152,17 @@ static void rockchip_dp_drm_encoder_enable(struct 
drm_encoder *encoder)
 
dev_dbg(dp->dev, "vop %s output to dp\n", (ret) ? "LIT" : "BIG");
 
-   ret = regmap_write(dp->grf, dp->data->lcdsel_grf_reg, val);
-   if (ret != 0) {
-   dev_err(dp->dev, "Could not write to GRF: %d\n", ret);
+   ret = clk_prepare_enable(dp->grfclk);
+   if (ret < 0) {
+   dev_err(dp->dev, "failed to enable grfclk %d\n", ret);
return;
}
+
+   ret = regmap_write(dp->grf, dp->data->lcdsel_grf_reg, val);
+   if (ret != 0)
+   dev_err(dp->dev, "Could not write to GRF: %d\n", ret);
+
+   clk_disable_unprepare(dp->grfclk);
 }
 
 static void rockchip_dp_drm_encoder_nop(struct drm_encoder *encoder)
@@ -235,6 +242,16 @@ static int rockchip_dp_init(struct rockchip_dp_device *dp)
return PTR_ERR(dp->grf);
}
 
+   dp->grfclk = devm_clk_get(dev, "grf");
+   if (PTR_ERR(dp->grfclk) == -ENOENT) {
+   dp->grfclk = NULL;
+   } else if (PTR_ERR(dp->grfclk) == -EPROBE_DEFER) {
+   return -EPROBE_DEFER;
+   } else if (IS_ERR(dp->grfclk)) {
+   dev_err(dev, "failed to get grf clock\n");
+   return PTR_ERR(dp->grfclk);
+   }
+
dp->pclk = devm_clk_get(dev, "pclk");
if (IS_ERR(dp->pclk)) {
dev_err(dev, "failed to get pclk property\n");
-- 
1.9.1