drm/tilcdc: Configure video mode to HW in enable() not in mode_set_nofb()

Configure video mode to HW in enable() call back. There is no reason
to do it before that. This makes PM functions way easier because there
is no HW context to save when screen is for instance blanked. This
patch removes mode_set_nofb() call back from tilcdc.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
Tested-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

drivers/gpu/drm/tilcdc/tilcdc_crtc.c

@@ -215,214 +215,6 @@ static void reset(struct drm_crtc *crtc)
 	tilcdc_clear(dev, LCDC_CLK_RESET_REG, LCDC_CLK_MAIN_RESET);
 }
 
-static void tilcdc_crtc_enable(struct drm_crtc *crtc)
-{
-	struct drm_device *dev = crtc->dev;
-	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
-
-	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
-	mutex_lock(&tilcdc_crtc->enable_lock);
-	if (tilcdc_crtc->enabled || tilcdc_crtc->shutdown) {
-		mutex_unlock(&tilcdc_crtc->enable_lock);
-		return;
-	}
-
-	pm_runtime_get_sync(dev->dev);
-
-	reset(crtc);
-
-	tilcdc_crtc_enable_irqs(dev);
-
-	tilcdc_clear(dev, LCDC_DMA_CTRL_REG, LCDC_DUAL_FRAME_BUFFER_ENABLE);
-	tilcdc_write_mask(dev, LCDC_RASTER_CTRL_REG,
-			  LCDC_PALETTE_LOAD_MODE(DATA_ONLY),
-			  LCDC_PALETTE_LOAD_MODE_MASK);
-	tilcdc_set(dev, LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE);
-
-	drm_crtc_vblank_on(crtc);
-
-	tilcdc_crtc->enabled = true;
-	mutex_unlock(&tilcdc_crtc->enable_lock);
-}
-
-static void tilcdc_crtc_off(struct drm_crtc *crtc, bool shutdown)
-{
-	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
-	struct drm_device *dev = crtc->dev;
-	struct tilcdc_drm_private *priv = dev->dev_private;
-
-	mutex_lock(&tilcdc_crtc->enable_lock);
-	if (shutdown)
-		tilcdc_crtc->shutdown = true;
-	if (!tilcdc_crtc->enabled) {
-		mutex_unlock(&tilcdc_crtc->enable_lock);
-		return;
-	}
-	tilcdc_crtc->frame_done = false;
-	tilcdc_clear(dev, LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE);
-
-	/*
-	 * if necessary wait for framedone irq which will still come
-	 * before putting things to sleep..
-	 */
-	if (priv->rev == 2) {
-		int ret = wait_event_timeout(tilcdc_crtc->frame_done_wq,
-					     tilcdc_crtc->frame_done,
-					     msecs_to_jiffies(500));
-		if (ret == 0)
-			dev_err(dev->dev, "%s: timeout waiting for framedone\n",
-				__func__);
-	}
-
-	drm_crtc_vblank_off(crtc);
-
-	tilcdc_crtc_disable_irqs(dev);
-
-	pm_runtime_put_sync(dev->dev);
-
-	if (tilcdc_crtc->next_fb) {
-		drm_flip_work_queue(&tilcdc_crtc->unref_work,
-				    tilcdc_crtc->next_fb);
-		tilcdc_crtc->next_fb = NULL;
-	}
-
-	if (tilcdc_crtc->curr_fb) {
-		drm_flip_work_queue(&tilcdc_crtc->unref_work,
-				    tilcdc_crtc->curr_fb);
-		tilcdc_crtc->curr_fb = NULL;
-	}
-
-	drm_flip_work_commit(&tilcdc_crtc->unref_work, priv->wq);
-	tilcdc_crtc->last_vblank = ktime_set(0, 0);
-
-	tilcdc_crtc->enabled = false;
-	mutex_unlock(&tilcdc_crtc->enable_lock);
-}
-
-static void tilcdc_crtc_disable(struct drm_crtc *crtc)
-{
-	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
-	tilcdc_crtc_off(crtc, false);
-}
-
-void tilcdc_crtc_shutdown(struct drm_crtc *crtc)
-{
-	tilcdc_crtc_off(crtc, true);
-}
-
-static bool tilcdc_crtc_is_on(struct drm_crtc *crtc)
-{
-	return crtc->state && crtc->state->enable && crtc->state->active;
-}
-
-static void tilcdc_crtc_recover_work(struct work_struct *work)
-{
-	struct tilcdc_crtc *tilcdc_crtc =
-		container_of(work, struct tilcdc_crtc, recover_work);
-	struct drm_crtc *crtc = &tilcdc_crtc->base;
-
-	dev_info(crtc->dev->dev, "%s: Reset CRTC", __func__);
-
-	drm_modeset_lock_crtc(crtc, NULL);
-
-	if (!tilcdc_crtc_is_on(crtc))
-		goto out;
-
-	tilcdc_crtc_disable(crtc);
-	tilcdc_crtc_enable(crtc);
-out:
-	drm_modeset_unlock_crtc(crtc);
-}
-
-static void tilcdc_crtc_destroy(struct drm_crtc *crtc)
-{
-	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
-	struct tilcdc_drm_private *priv = crtc->dev->dev_private;
-
-	drm_modeset_lock_crtc(crtc, NULL);
-	tilcdc_crtc_disable(crtc);
-	drm_modeset_unlock_crtc(crtc);
-
-	flush_workqueue(priv->wq);
-
-	of_node_put(crtc->port);
-	drm_crtc_cleanup(crtc);
-	drm_flip_work_cleanup(&tilcdc_crtc->unref_work);
-}
-
-int tilcdc_crtc_update_fb(struct drm_crtc *crtc,
-		struct drm_framebuffer *fb,
-		struct drm_pending_vblank_event *event)
-{
-	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
-	struct drm_device *dev = crtc->dev;
-	unsigned long flags;
-
-	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
-
-	if (tilcdc_crtc->event) {
-		dev_err(dev->dev, "already pending page flip!\n");
-		return -EBUSY;
-	}
-
-	drm_framebuffer_reference(fb);
-
-	crtc->primary->fb = fb;
-
-	spin_lock_irqsave(&tilcdc_crtc->irq_lock, flags);
-
-	if (crtc->hwmode.vrefresh && ktime_to_ns(tilcdc_crtc->last_vblank)) {
-		ktime_t next_vblank;
-		s64 tdiff;
-
-		next_vblank = ktime_add_us(tilcdc_crtc->last_vblank,
-			1000000 / crtc->hwmode.vrefresh);
-
-		tdiff = ktime_to_us(ktime_sub(next_vblank, ktime_get()));
-
-		if (tdiff < TILCDC_VBLANK_SAFETY_THRESHOLD_US)
-			tilcdc_crtc->next_fb = fb;
-	}
-
-	if (tilcdc_crtc->next_fb != fb)
-		set_scanout(crtc, fb);
-
-	tilcdc_crtc->event = event;
-
-	spin_unlock_irqrestore(&tilcdc_crtc->irq_lock, flags);
-
-	return 0;
-}
-
-static bool tilcdc_crtc_mode_fixup(struct drm_crtc *crtc,
-		const struct drm_display_mode *mode,
-		struct drm_display_mode *adjusted_mode)
-{
-	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
-
-	if (!tilcdc_crtc->simulate_vesa_sync)
-		return true;
-
-	/*
-	 * tilcdc does not generate VESA-compliant sync but aligns
-	 * VS on the second edge of HS instead of first edge.
-	 * We use adjusted_mode, to fixup sync by aligning both rising
-	 * edges and add HSKEW offset to fix the sync.
-	 */
-	adjusted_mode->hskew = mode->hsync_end - mode->hsync_start;
-	adjusted_mode->flags |= DRM_MODE_FLAG_HSKEW;
-
-	if (mode->flags & DRM_MODE_FLAG_NHSYNC) {
-		adjusted_mode->flags |= DRM_MODE_FLAG_PHSYNC;
-		adjusted_mode->flags &= ~DRM_MODE_FLAG_NHSYNC;
-	} else {
-		adjusted_mode->flags |= DRM_MODE_FLAG_NHSYNC;
-		adjusted_mode->flags &= ~DRM_MODE_FLAG_PHSYNC;
-	}
-
-	return true;
-}
-
 /*
  * Calculate the percentage difference between the requested pixel clock rate
  * and the effective rate resulting from calculating the clock divider value.
@@ -499,7 +291,7 @@ static void tilcdc_crtc_set_clk(struct drm_crtc *crtc)
 				LCDC_V2_CORE_CLK_EN);
 }
 
-static void tilcdc_crtc_mode_set_nofb(struct drm_crtc *crtc)
+static void tilcdc_crtc_set_mode(struct drm_crtc *crtc)
 {
 	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
@@ -509,8 +301,6 @@ static void tilcdc_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	struct drm_display_mode *mode = &crtc->state->adjusted_mode;
 	struct drm_framebuffer *fb = crtc->primary->state->fb;
 
-	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
-
 	if (WARN_ON(!info))
 		return;
 
@@ -659,17 +449,226 @@ static void tilcdc_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	else
 		tilcdc_clear(dev, LCDC_RASTER_CTRL_REG, LCDC_RASTER_ORDER);
 
-	drm_framebuffer_reference(fb);
-
 	tilcdc_crtc_set_clk(crtc);
 
 	tilcdc_crtc_load_palette(crtc);
 
 	set_scanout(crtc, fb);
 
+	drm_framebuffer_reference(fb);
+
 	crtc->hwmode = crtc->state->adjusted_mode;
 }
 
+static void tilcdc_crtc_enable(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
+
+	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
+	mutex_lock(&tilcdc_crtc->enable_lock);
+	if (tilcdc_crtc->enabled || tilcdc_crtc->shutdown) {
+		mutex_unlock(&tilcdc_crtc->enable_lock);
+		return;
+	}
+
+	pm_runtime_get_sync(dev->dev);
+
+	reset(crtc);
+
+	tilcdc_crtc_set_mode(crtc);
+
+	tilcdc_crtc_enable_irqs(dev);
+
+	tilcdc_clear(dev, LCDC_DMA_CTRL_REG, LCDC_DUAL_FRAME_BUFFER_ENABLE);
+	tilcdc_write_mask(dev, LCDC_RASTER_CTRL_REG,
+			  LCDC_PALETTE_LOAD_MODE(DATA_ONLY),
+			  LCDC_PALETTE_LOAD_MODE_MASK);
+	tilcdc_set(dev, LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE);
+
+	drm_crtc_vblank_on(crtc);
+
+	tilcdc_crtc->enabled = true;
+	mutex_unlock(&tilcdc_crtc->enable_lock);
+}
+
+static void tilcdc_crtc_off(struct drm_crtc *crtc, bool shutdown)
+{
+	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
+	struct drm_device *dev = crtc->dev;
+	struct tilcdc_drm_private *priv = dev->dev_private;
+	int ret;
+
+	mutex_lock(&tilcdc_crtc->enable_lock);
+	if (shutdown)
+		tilcdc_crtc->shutdown = true;
+	if (!tilcdc_crtc->enabled) {
+		mutex_unlock(&tilcdc_crtc->enable_lock);
+		return;
+	}
+	tilcdc_crtc->frame_done = false;
+	tilcdc_clear(dev, LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE);
+
+	/*
+	 * Wait for framedone irq which will still come before putting
+	 * things to sleep..
+	 */
+	ret = wait_event_timeout(tilcdc_crtc->frame_done_wq,
+				 tilcdc_crtc->frame_done,
+				 msecs_to_jiffies(500));
+	if (ret == 0)
+		dev_err(dev->dev, "%s: timeout waiting for framedone\n",
+			__func__);
+
+	drm_crtc_vblank_off(crtc);
+
+	tilcdc_crtc_disable_irqs(dev);
+
+	pm_runtime_put_sync(dev->dev);
+
+	if (tilcdc_crtc->next_fb) {
+		drm_flip_work_queue(&tilcdc_crtc->unref_work,
+				    tilcdc_crtc->next_fb);
+		tilcdc_crtc->next_fb = NULL;
+	}
+
+	if (tilcdc_crtc->curr_fb) {
+		drm_flip_work_queue(&tilcdc_crtc->unref_work,
+				    tilcdc_crtc->curr_fb);
+		tilcdc_crtc->curr_fb = NULL;
+	}
+
+	drm_flip_work_commit(&tilcdc_crtc->unref_work, priv->wq);
+	tilcdc_crtc->last_vblank = ktime_set(0, 0);
+
+	tilcdc_crtc->enabled = false;
+	mutex_unlock(&tilcdc_crtc->enable_lock);
+}
+
+static void tilcdc_crtc_disable(struct drm_crtc *crtc)
+{
+	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
+	tilcdc_crtc_off(crtc, false);
+}
+
+void tilcdc_crtc_shutdown(struct drm_crtc *crtc)
+{
+	tilcdc_crtc_off(crtc, true);
+}
+
+static bool tilcdc_crtc_is_on(struct drm_crtc *crtc)
+{
+	return crtc->state && crtc->state->enable && crtc->state->active;
+}
+
+static void tilcdc_crtc_recover_work(struct work_struct *work)
+{
+	struct tilcdc_crtc *tilcdc_crtc =
+		container_of(work, struct tilcdc_crtc, recover_work);
+	struct drm_crtc *crtc = &tilcdc_crtc->base;
+
+	dev_info(crtc->dev->dev, "%s: Reset CRTC", __func__);
+
+	drm_modeset_lock_crtc(crtc, NULL);
+
+	if (!tilcdc_crtc_is_on(crtc))
+		goto out;
+
+	tilcdc_crtc_disable(crtc);
+	tilcdc_crtc_enable(crtc);
+out:
+	drm_modeset_unlock_crtc(crtc);
+}
+
+static void tilcdc_crtc_destroy(struct drm_crtc *crtc)
+{
+	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
+	struct tilcdc_drm_private *priv = crtc->dev->dev_private;
+
+	drm_modeset_lock_crtc(crtc, NULL);
+	tilcdc_crtc_disable(crtc);
+	drm_modeset_unlock_crtc(crtc);
+
+	flush_workqueue(priv->wq);
+
+	of_node_put(crtc->port);
+	drm_crtc_cleanup(crtc);
+	drm_flip_work_cleanup(&tilcdc_crtc->unref_work);
+}
+
+int tilcdc_crtc_update_fb(struct drm_crtc *crtc,
+		struct drm_framebuffer *fb,
+		struct drm_pending_vblank_event *event)
+{
+	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
+	struct drm_device *dev = crtc->dev;
+	unsigned long flags;
+
+	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
+
+	if (tilcdc_crtc->event) {
+		dev_err(dev->dev, "already pending page flip!\n");
+		return -EBUSY;
+	}
+
+	drm_framebuffer_reference(fb);
+
+	crtc->primary->fb = fb;
+
+	spin_lock_irqsave(&tilcdc_crtc->irq_lock, flags);
+
+	if (crtc->hwmode.vrefresh && ktime_to_ns(tilcdc_crtc->last_vblank)) {
+		ktime_t next_vblank;
+		s64 tdiff;
+
+		next_vblank = ktime_add_us(tilcdc_crtc->last_vblank,
+			1000000 / crtc->hwmode.vrefresh);
+
+		tdiff = ktime_to_us(ktime_sub(next_vblank, ktime_get()));
+
+		if (tdiff < TILCDC_VBLANK_SAFETY_THRESHOLD_US)
+			tilcdc_crtc->next_fb = fb;
+	}
+
+	if (tilcdc_crtc->next_fb != fb)
+		set_scanout(crtc, fb);
+
+	tilcdc_crtc->event = event;
+
+	spin_unlock_irqrestore(&tilcdc_crtc->irq_lock, flags);
+
+	return 0;
+}
+
+static bool tilcdc_crtc_mode_fixup(struct drm_crtc *crtc,
+		const struct drm_display_mode *mode,
+		struct drm_display_mode *adjusted_mode)
+{
+	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
+
+	if (!tilcdc_crtc->simulate_vesa_sync)
+		return true;
+
+	/*
+	 * tilcdc does not generate VESA-compliant sync but aligns
+	 * VS on the second edge of HS instead of first edge.
+	 * We use adjusted_mode, to fixup sync by aligning both rising
+	 * edges and add HSKEW offset to fix the sync.
+	 */
+	adjusted_mode->hskew = mode->hsync_end - mode->hsync_start;
+	adjusted_mode->flags |= DRM_MODE_FLAG_HSKEW;
+
+	if (mode->flags & DRM_MODE_FLAG_NHSYNC) {
+		adjusted_mode->flags |= DRM_MODE_FLAG_PHSYNC;
+		adjusted_mode->flags &= ~DRM_MODE_FLAG_NHSYNC;
+	} else {
+		adjusted_mode->flags |= DRM_MODE_FLAG_NHSYNC;
+		adjusted_mode->flags &= ~DRM_MODE_FLAG_PHSYNC;
+	}
+
+	return true;
+}
+
 static int tilcdc_crtc_atomic_check(struct drm_crtc *crtc,
 				    struct drm_crtc_state *state)
 {
@@ -710,7 +709,6 @@ static const struct drm_crtc_helper_funcs tilcdc_crtc_helper_funcs = {
 		.enable		= tilcdc_crtc_enable,
 		.disable	= tilcdc_crtc_disable,
 		.atomic_check	= tilcdc_crtc_atomic_check,
-		.mode_set_nofb	= tilcdc_crtc_mode_set_nofb,
 };
 
 int tilcdc_crtc_max_width(struct drm_crtc *crtc)

drivers/gpu/drm/tilcdc/tilcdc_drv.c

@@ -127,18 +127,12 @@ static int tilcdc_commit(struct drm_device *dev,
 	 * current layout.
 	 */
 
-	/* Keep HW on while we commit the state. */
-	pm_runtime_get_sync(dev->dev);
-
 	drm_atomic_helper_commit_modeset_disables(dev, state);
 
 	drm_atomic_helper_commit_planes(dev, state, 0);
 
 	drm_atomic_helper_commit_modeset_enables(dev, state);
 
-	/* Now HW should remain on if need becase the crtc is enabled */
-	pm_runtime_put_sync(dev->dev);
-
 	drm_atomic_helper_wait_for_vblanks(dev, state);
 
 	drm_atomic_helper_cleanup_planes(dev, state);