[PATCH v2 0/3] drm/tilcdc: fix LCD pixel clock setting

[PATCH v2 0/3] drm/tilcdc: fix LCD pixel clock setting

[Dario Binacchi]

The series was born from a patch to fix the LCD pixel clock setting.
Two additional patches have been added to this. One renames a misleading
variable name that was probably the cause of the bug and the other fixes
a warning message.


Changes in v2:
- The patch has been added in version 2.
- Rename clk_div_rate to real_pclk_rate.
- Provide pixel clock rate to tilcdc_pclk_diff().
- The patch has been added in version 2.

Dario Binacchi (3):
  drm/tilcdc: rename req_rate to pclk_rate
  drm/tilcdc: fix LCD pixel clock setting
  drm/tilcdc: fix pixel clock setting warning message

 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

-- 
2.17.1

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

[PATCH v2 1/3] drm/tilcdc: rename req_rate to pclk_rate

[Dario Binacchi]

The req_rate name is a little misleading, so let's rename to pclk_rate
(pixel clock rate).

Signed-off-by: Dario Binacchi <dariobin@libero.it>

---

Changes in v2:
- The patch has been added in version 2.

 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 30213708fc99..aeec5786617d 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -203,18 +203,18 @@ static void tilcdc_crtc_set_clk(struct drm_crtc *crtc)
 	struct drm_device *dev = crtc->dev;
 	struct tilcdc_drm_private *priv = dev->dev_private;
 	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
-	unsigned long clk_rate, real_rate, req_rate;
+	unsigned long clk_rate, real_rate, pclk_rate;
 	unsigned int clkdiv;
 	int ret;
 
 	clkdiv = 2; /* first try using a standard divider of 2 */
 
 	/* mode.clock is in KHz, set_rate wants parameter in Hz */
-	req_rate = crtc->mode.clock * 1000;
+	pclk_rate = crtc->mode.clock * 1000;
 
-	ret = clk_set_rate(priv->clk, req_rate * clkdiv);
+	ret = clk_set_rate(priv->clk, pclk_rate * clkdiv);
 	clk_rate = clk_get_rate(priv->clk);
-	if (ret < 0 || tilcdc_pclk_diff(req_rate, clk_rate) > 5) {
+	if (ret < 0 || tilcdc_pclk_diff(pclk_rate, clk_rate) > 5) {
 		/*
 		 * If we fail to set the clock rate (some architectures don't
 		 * use the common clock framework yet and may not implement
@@ -229,7 +229,7 @@ static void tilcdc_crtc_set_clk(struct drm_crtc *crtc)
 			return;
 		}
 
-		clkdiv = DIV_ROUND_CLOSEST(clk_rate, req_rate);
+		clkdiv = DIV_ROUND_CLOSEST(clk_rate, pclk_rate);
 
 		/*
 		 * Emit a warning if the real clock rate resulting from the
@@ -238,7 +238,7 @@ static void tilcdc_crtc_set_clk(struct drm_crtc *crtc)
 		 * 5% is an arbitrary value - LCDs are usually quite tolerant
 		 * about pixel clock rates.
 		 */
-		real_rate = clkdiv * req_rate;
+		real_rate = clkdiv * pclk_rate;
 
 		if (tilcdc_pclk_diff(clk_rate, real_rate) > 5) {
 			dev_warn(dev->dev,
-- 
2.17.1

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

[PATCH v2 2/3] drm/tilcdc: fix LCD pixel clock setting

[Dario Binacchi]

The tilcdc_pclk_diff() compares the requested pixel clock rate to the
real one, so passing it clk_rate instead of clk_rate / clkdiv caused
it to fail even if the clk_rate was properly set. Adding the
real_pclk_rate variable makes the code more readable.

Signed-off-by: Dario Binacchi <dariobin@libero.it>

---

Changes in v2:
- Rename clk_div_rate to real_pclk_rate.
- Provide pixel clock rate to tilcdc_pclk_diff().

 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index aeec5786617d..ac6228cb04d9 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -203,7 +203,7 @@ static void tilcdc_crtc_set_clk(struct drm_crtc *crtc)
 	struct drm_device *dev = crtc->dev;
 	struct tilcdc_drm_private *priv = dev->dev_private;
 	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
-	unsigned long clk_rate, real_rate, pclk_rate;
+	unsigned long clk_rate, real_rate, real_pclk_rate, pclk_rate;
 	unsigned int clkdiv;
 	int ret;
 
@@ -214,7 +214,8 @@ static void tilcdc_crtc_set_clk(struct drm_crtc *crtc)
 
 	ret = clk_set_rate(priv->clk, pclk_rate * clkdiv);
 	clk_rate = clk_get_rate(priv->clk);
-	if (ret < 0 || tilcdc_pclk_diff(pclk_rate, clk_rate) > 5) {
+	real_pclk_rate = clk_rate / clkdiv;
+	if (ret < 0 || tilcdc_pclk_diff(pclk_rate, real_pclk_rate) > 5) {
 		/*
 		 * If we fail to set the clock rate (some architectures don't
 		 * use the common clock framework yet and may not implement
-- 
2.17.1

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

[PATCH v2 3/3] drm/tilcdc: fix pixel clock setting warning message

[Dario Binacchi]

The warning message did not printed the LCD pixel clock rate but the LCD
clock divisor input rate. As a consequence, the required and real pixel
clock rates are now passed to the tilcdc_pclk_diff().

Signed-off-by: Dario Binacchi <dariobin@libero.it>

---

Changes in v2:
- The patch has been added in version 2.

 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index ac6228cb04d9..c0792c52dc02 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -203,7 +203,7 @@ static void tilcdc_crtc_set_clk(struct drm_crtc *crtc)
 	struct drm_device *dev = crtc->dev;
 	struct tilcdc_drm_private *priv = dev->dev_private;
 	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
-	unsigned long clk_rate, real_rate, real_pclk_rate, pclk_rate;
+	unsigned long clk_rate, real_pclk_rate, pclk_rate;
 	unsigned int clkdiv;
 	int ret;
 
@@ -239,12 +239,12 @@ static void tilcdc_crtc_set_clk(struct drm_crtc *crtc)
 		 * 5% is an arbitrary value - LCDs are usually quite tolerant
 		 * about pixel clock rates.
 		 */
-		real_rate = clkdiv * pclk_rate;
+		real_pclk_rate = clk_rate / clkdiv;
 
-		if (tilcdc_pclk_diff(clk_rate, real_rate) > 5) {
+		if (tilcdc_pclk_diff(pclk_rate, real_pclk_rate) > 5) {
 			dev_warn(dev->dev,
 				 "effective pixel clock rate (%luHz) differs from the calculated rate (%luHz)\n",
-				 clk_rate, real_rate);
+				 pclk_rate, real_pclk_rate);
 		}
 	}
 
-- 
2.17.1

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

Re: [PATCH v2 0/3] drm/tilcdc: fix LCD pixel clock setting

[Jyri Sarha]

On 2021-03-21 10:31, Dario Binacchi wrote:
> The series was born from a patch to fix the LCD pixel clock setting.
> Two additional patches have been added to this. One renames a 
> misleading
> variable name that was probably the cause of the bug and the other 
> fixes
> a warning message.
> 

Thanks you,

I think this looks good now.

Reviewed-by: Jyri Sarha <jyri.sarha@iki.fi>

For the series.

I'll wait a day or two if Tomi has something more to say and merge this 
to drm-misc-next.

Best regards,
Jyri

> 
> Changes in v2:
> - The patch has been added in version 2.
> - Rename clk_div_rate to real_pclk_rate.
> - Provide pixel clock rate to tilcdc_pclk_diff().
> - The patch has been added in version 2.
> 
> Dario Binacchi (3):
>   drm/tilcdc: rename req_rate to pclk_rate
>   drm/tilcdc: fix LCD pixel clock setting
>   drm/tilcdc: fix pixel clock setting warning message
> 
>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 3/3] drm/tilcdc: fix pixel clock setting warning message

[Tomi Valkeinen]

On 21/03/2021 10:31, Dario Binacchi wrote:
> The warning message did not printed the LCD pixel clock rate but the LCD
> clock divisor input rate. As a consequence, the required and real pixel
> clock rates are now passed to the tilcdc_pclk_diff().
> 
> Signed-off-by: Dario Binacchi <dariobin@libero.it>
> 
> ---
> 
> Changes in v2:
> - The patch has been added in version 2.
> 
>   drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> index ac6228cb04d9..c0792c52dc02 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> @@ -203,7 +203,7 @@ static void tilcdc_crtc_set_clk(struct drm_crtc *crtc)
>   	struct drm_device *dev = crtc->dev;
>   	struct tilcdc_drm_private *priv = dev->dev_private;
>   	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
> -	unsigned long clk_rate, real_rate, real_pclk_rate, pclk_rate;
> +	unsigned long clk_rate, real_pclk_rate, pclk_rate;
>   	unsigned int clkdiv;
>   	int ret;
>   
> @@ -239,12 +239,12 @@ static void tilcdc_crtc_set_clk(struct drm_crtc *crtc)
>   		 * 5% is an arbitrary value - LCDs are usually quite tolerant
>   		 * about pixel clock rates.
>   		 */
> -		real_rate = clkdiv * pclk_rate;
> +		real_pclk_rate = clk_rate / clkdiv;
>   
> -		if (tilcdc_pclk_diff(clk_rate, real_rate) > 5) {
> +		if (tilcdc_pclk_diff(pclk_rate, real_pclk_rate) > 5) {
>   			dev_warn(dev->dev,
>   				 "effective pixel clock rate (%luHz) differs from the calculated rate (%luHz)\n",
> -				 clk_rate, real_rate);
> +				 pclk_rate, real_pclk_rate);

Aren't these backwards? "Effective" is the real one in the HW. I'm not 
sure what "calculated" means here, I guess it should be "requested".

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

Re: [PATCH v2 0/3] drm/tilcdc: fix LCD pixel clock setting

[Tomi Valkeinen]

On 21/03/2021 21:08, Jyri Sarha wrote:
> On 2021-03-21 10:31, Dario Binacchi wrote:
>> The series was born from a patch to fix the LCD pixel clock setting.
>> Two additional patches have been added to this. One renames a misleading
>> variable name that was probably the cause of the bug and the other fixes
>> a warning message.
>>
> 
> Thanks you,
> 
> I think this looks good now.
> 
> Reviewed-by: Jyri Sarha <jyri.sarha@iki.fi>
> 
> For the series.
> 
> I'll wait a day or two if Tomi has something more to say and merge this 
> to drm-misc-next.

I had one comment about the print, but otherwise:

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

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