diff mbox

[2/3] drm/tilcdc: Add tilcdc_crtc_set_clk() and cleanup cpufreq_transition()

Message ID eb7d809e40f8909b0d2c0e05c4722d8742b8a394.1473148345.git.jsarha@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jyri Sarha Sept. 6, 2016, 8:19 a.m. UTC 
Add tilcdc_crtc_set_clk() and cleanup cpufreq_transition(). The new
tilcdc_crtc_set_clk() is used in tilcdc_crtc_mode_set_nofb() instead
tilcdc_crtc_update_clk(). New tilcdc_crtc_update_clk() is implemented
using tilcdc_crtc_set_clk() for cpufreq_transition() alone.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 78 +++++++++++++++++++++---------------
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  | 14 ++-----
 drivers/gpu/drm/tilcdc/tilcdc_drv.h  |  1 -
 3 files changed, 49 insertions(+), 44 deletions(-)

Comments

Tomi Valkeinen Sept. 6, 2016, 9:46 a.m. UTC | #1 
On 06/09/16 11:19, Jyri Sarha wrote:
> Add tilcdc_crtc_set_clk() and cleanup cpufreq_transition(). The new
> tilcdc_crtc_set_clk() is used in tilcdc_crtc_mode_set_nofb() instead
> tilcdc_crtc_update_clk(). New tilcdc_crtc_update_clk() is implemented
> using tilcdc_crtc_set_clk() for cpufreq_transition() alone.

I think the patch looks ok, but the description doesn't help too much in
understanding what this does. It gives no hint on what
tilcdc_crtc_set_clk() is, what's its relation to
tilcdc_crtc_update_clk(), and what's the overall purpose here.

If I have to read the code to understand the reason for the patch, the
description has failed =).

 Tomi
Tomi Valkeinen Sept. 6, 2016, 9:48 a.m. UTC | #2 
On 06/09/16 11:19, Jyri Sarha wrote:

> +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);
> +	const unsigned clkdiv = 2; /* using a fixed divider of 2 */

Btw, not related to this patch, but you may want to revisit this
hardcoded divider at some point. I don't remember the history, but it
was a quick solution to some issues. Supporting any divider the HW
allows would probably give us better clock rates.

Well, except if the source clock comes right from a dedicated PLL, which
supports the whole pixel clock range. If that's the case, then the LCDC
divider doesn't help much.

 Tomi
diff mbox

Patch

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 84b36fd..f9e3da9 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -35,6 +35,8 @@  struct tilcdc_crtc {
 	bool frame_done;
 	spinlock_t irq_lock;
 
+	unsigned int lcd_fck_rate;
+
 	ktime_t last_vblank;
 
 	struct drm_framebuffer *curr_fb;
@@ -324,6 +326,37 @@  static bool tilcdc_crtc_mode_fixup(struct drm_crtc *crtc,
 	return true;
 }
 
+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);
+	const unsigned clkdiv = 2; /* using a fixed divider of 2 */
+	int ret;
+
+	/* mode.clock is in KHz, set_rate wants parameter in Hz */
+	ret = clk_set_rate(priv->clk, crtc->mode.clock * 1000 * clkdiv);
+	if (ret < 0) {
+		dev_err(dev->dev, "failed to set display clock rate to: %d\n",
+			crtc->mode.clock);
+		return;
+	}
+
+	tilcdc_crtc->lcd_fck_rate = clk_get_rate(priv->clk);
+
+	DBG("lcd_clk=%u, mode clock=%d, div=%u",
+	    tilcdc_crtc->lcd_fck_rate, crtc->mode.clock, clkdiv);
+
+	/* Configure the LCD clock divisor. */
+	tilcdc_write(dev, LCDC_CTRL_REG, LCDC_CLK_DIVISOR(clkdiv) |
+		     LCDC_RASTER_MODE);
+
+	if (priv->rev == 2)
+		tilcdc_set(dev, LCDC_CLK_ENABLE_REG,
+				LCDC_V2_DMA_CLK_EN | LCDC_V2_LIDD_CLK_EN |
+				LCDC_V2_CORE_CLK_EN);
+}
+
 static void tilcdc_crtc_mode_set_nofb(struct drm_crtc *crtc)
 {
 	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
@@ -486,7 +519,7 @@  static void tilcdc_crtc_mode_set_nofb(struct drm_crtc *crtc)
 
 	set_scanout(crtc, fb);
 
-	tilcdc_crtc_update_clk(crtc);
+	tilcdc_crtc_set_clk(crtc);
 
 	crtc->hwmode = crtc->state->adjusted_mode;
 }
@@ -655,41 +688,22 @@  void tilcdc_crtc_update_clk(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	struct tilcdc_drm_private *priv = dev->dev_private;
-	unsigned long lcd_clk;
-	const unsigned clkdiv = 2; /* using a fixed divider of 2 */
-	int ret;
+	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
+	struct drm_mode_config *config = &priv->dev->mode_config;
 
-	pm_runtime_get_sync(dev->dev);
+	if (tilcdc_crtc->lcd_fck_rate != clk_get_rate(priv->clk)) {
+		mutex_lock(&config->mutex);
+		if (tilcdc_crtc_is_on(crtc)) {
+			pm_runtime_get_sync(dev->dev);
+			tilcdc_crtc_disable(crtc);
 
-	tilcdc_crtc_disable(crtc);
+			tilcdc_crtc_set_clk(crtc);
 
-	/* mode.clock is in KHz, set_rate wants parameter in Hz */
-	ret = clk_set_rate(priv->clk, crtc->mode.clock * 1000 * clkdiv);
-	if (ret < 0) {
-		dev_err(dev->dev, "failed to set display clock rate to: %d\n",
-				crtc->mode.clock);
-		goto out;
+			tilcdc_crtc_enable(crtc);
+			pm_runtime_put_sync(dev->dev);
+		}
+		mutex_unlock(&config->mutex);
 	}
-
-	lcd_clk = clk_get_rate(priv->clk);
-
-	DBG("lcd_clk=%lu, mode clock=%d, div=%u",
-		lcd_clk, crtc->mode.clock, clkdiv);
-
-	/* Configure the LCD clock divisor. */
-	tilcdc_write(dev, LCDC_CTRL_REG, LCDC_CLK_DIVISOR(clkdiv) |
-			LCDC_RASTER_MODE);
-
-	if (priv->rev == 2)
-		tilcdc_set(dev, LCDC_CLK_ENABLE_REG,
-				LCDC_V2_DMA_CLK_EN | LCDC_V2_LIDD_CLK_EN |
-				LCDC_V2_CORE_CLK_EN);
-
-	if (tilcdc_crtc_is_on(crtc))
-		tilcdc_crtc_enable(crtc);
-
-out:
-	pm_runtime_put_sync(dev->dev);
 }
 
 #define SYNC_LOST_COUNT_LIMIT 50
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 882d9b5..4af58b7 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -184,16 +184,9 @@  static int cpufreq_transition(struct notifier_block *nb,
 {
 	struct tilcdc_drm_private *priv = container_of(nb,
 			struct tilcdc_drm_private, freq_transition);
-	struct drm_mode_config *config = &priv->dev->mode_config;
-
-	if (val == CPUFREQ_POSTCHANGE) {
-		if (priv->lcd_fck_rate != clk_get_rate(priv->clk)) {
-			mutex_lock(&config->mutex);
-			priv->lcd_fck_rate = clk_get_rate(priv->clk);
-			tilcdc_crtc_update_clk(priv->crtc);
-			mutex_unlock(&config->mutex);
-		}
-	}
+
+	if (val == CPUFREQ_POSTCHANGE)
+		tilcdc_crtc_update_clk(priv->crtc);
 
 	return 0;
 }
@@ -288,7 +281,6 @@  static int tilcdc_load(struct drm_device *dev, unsigned long flags)
 	}
 
 #ifdef CONFIG_CPU_FREQ
-	priv->lcd_fck_rate = clk_get_rate(priv->clk);
 	priv->freq_transition.notifier_call = cpufreq_transition;
 	ret = cpufreq_register_notifier(&priv->freq_transition,
 			CPUFREQ_TRANSITION_NOTIFIER);
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
index 6caecfc..fac0733 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
@@ -76,7 +76,6 @@  struct tilcdc_drm_private {
 
 #ifdef CONFIG_CPU_FREQ
 	struct notifier_block freq_transition;
-	unsigned int lcd_fck_rate;
 #endif
 
 	struct workqueue_struct *wq;