[PATCH 00/12] drm/tilcdc: fixes and added features from TI tree

[PATCH 00/12] drm/tilcdc: fixes and added features from TI tree

[Jyri Sarha]

Most of these patches are quite old and they have been used in TI
linux for some time already. I forward ported the applicaple patches
from TI tree on top of the latest drm-next, fixed checkpatch issues
and tested the driver.

Darren Etheridge (8):
  drm/tilcdc: disable console switching during pm operations
  drm/tilcdc: rewrite pixel clock calculation
  drm/tilcdc: fix kernel panic on suspend when no hdmi monitor connected
  drm/tilcdc: make frame_done interrupt active at all times
  drm/tilcdc: disable the lcd controller/dma engine when suspend invoked
  drm/tilcdc: fix the ping-pong dma tearing issue seen on buffer
    flipping
  drm/tilcdc: correct the dmachannel tracking logic
  drm/tilcdc: make frame completion interrupt always enabled

Dave Gerlach (1):
  drm/tilcdc: adopt pinctrl support

Grygorii Strashko (1):
  drm/tilcdc: fix build error when !CONFIG_CPU_FREQ

Jyri Sarha (1):
  drm/tilcdc: Implement dma-buf support for tilcdc

Tomi Valkeinen (1):
  drm/tilcdc: verify fb pitch

 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 91 +++++++++++++++++++++++++++-------
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  | 94 +++++++++++++++++++++---------------
 drivers/gpu/drm/tilcdc/tilcdc_drv.h  |  3 +-
 3 files changed, 129 insertions(+), 59 deletions(-)

-- 
1.9.1

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

[PATCH 01/12] drm/tilcdc: disable console switching during pm operations

[Jyri Sarha]

From: Darren Etheridge <detheridge@ti.com>

The default behavior of consoles during power management operations is: On
entry to suspend a new console is allocated and switched to. On resume the
original console is restored.

However this isn't the observed behavior and the original console is not
restored. This commit avoids the problem by disabling the switching of
consoles at suspend/resume. This works because the driver already restores
all necessary hardware context during such pm operations.

Signed-off-by: Darren Etheridge <detheridge@ti.com>
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 4ddb21e..02525b2 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -18,6 +18,7 @@
 /* LCDC DRM driver, based on da8xx-fb */
 
 #include <linux/component.h>
+#include <linux/suspend.h>
 
 #include "tilcdc_drv.h"
 #include "tilcdc_regs.h"
@@ -229,6 +230,14 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
 	pm_runtime_enable(dev->dev);
 	pm_runtime_irq_safe(dev->dev);
 
+	/*
+	 * disable creation of new console during suspend.
+	 * this works around a problem where a ctrl-c is needed
+	 * to be entered on the VT to actually get the device
+	 * to continue into the suspend state.
+	 */
+	pm_set_vt_switch(0);
+
 	/* Determine LCD IP Version */
 	pm_runtime_get_sync(dev->dev);
 	switch (tilcdc_read(dev, LCDC_PID_REG)) {
-- 
1.9.1

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

[PATCH 02/12] drm/tilcdc: rewrite pixel clock calculation

[Jyri Sarha]

From: Darren Etheridge <detheridge@ti.com>

Updating the tilcdc DRM driver code to calculate the LCD controller
pixel clock more accurately. Based on a suggested implementation by
Tomi Valkeinen.

The current code does not work correctly and produces wrong results
with many requested clock rates. It also oddly uses two different
clocks, a display pll clock and a divider clock (child of display
pll), instead of just using the clock coming to the lcdc.

This patch removes the use of the display pll clock, and rewrites the
code to calculate the clock rates. The idea is simply to request a
clock rate of pixelclock*2, as the LCD controller has an internal
divider which we set to 2.

Signed-off-by: Darren Etheridge <detheridge@ti.com>
[Rewrapped description]
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 16 ++++++++--------
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  | 11 +----------
 drivers/gpu/drm/tilcdc/tilcdc_drv.h  |  1 -
 3 files changed, 9 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 7d07733..7b687ae 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -573,7 +573,8 @@ void tilcdc_crtc_update_clk(struct drm_crtc *crtc)
 	struct drm_device *dev = crtc->dev;
 	struct tilcdc_drm_private *priv = dev->dev_private;
 	int dpms = tilcdc_crtc->dpms;
-	unsigned int lcd_clk, div;
+	unsigned long lcd_clk;
+	const unsigned clkdiv = 2; /* using a fixed divider of 2 */
 	int ret;
 
 	pm_runtime_get_sync(dev->dev);
@@ -581,22 +582,21 @@ void tilcdc_crtc_update_clk(struct drm_crtc *crtc)
 	if (dpms == DRM_MODE_DPMS_ON)
 		tilcdc_crtc_dpms(crtc, DRM_MODE_DPMS_OFF);
 
-	/* in raster mode, minimum divisor is 2: */
-	ret = clk_set_rate(priv->disp_clk, crtc->mode.clock * 1000 * 2);
-	if (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);
 		goto out;
 	}
 
 	lcd_clk = clk_get_rate(priv->clk);
-	div = lcd_clk / (crtc->mode.clock * 1000);
 
-	DBG("lcd_clk=%u, mode clock=%d, div=%u", lcd_clk, crtc->mode.clock, div);
-	DBG("fck=%lu, dpll_disp_ck=%lu", clk_get_rate(priv->clk), clk_get_rate(priv->disp_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(div) |
+	tilcdc_write(dev, LCDC_CTRL_REG, LCDC_CLK_DIVISOR(clkdiv) |
 			LCDC_RASTER_MODE);
 
 	if (priv->rev == 2)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 02525b2..2b2780e 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -193,13 +193,6 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
 		goto fail_iounmap;
 	}
 
-	priv->disp_clk = clk_get(dev->dev, "dpll_disp_ck");
-	if (IS_ERR(priv->clk)) {
-		dev_err(dev->dev, "failed to get display clock\n");
-		ret = -ENODEV;
-		goto fail_put_clk;
-	}
-
 #ifdef CONFIG_CPU_FREQ
 	priv->lcd_fck_rate = clk_get_rate(priv->clk);
 	priv->freq_transition.notifier_call = cpufreq_transition;
@@ -207,7 +200,7 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
 			CPUFREQ_TRANSITION_NOTIFIER);
 	if (ret) {
 		dev_err(dev->dev, "failed to register cpufreq notifier\n");
-		goto fail_put_disp_clk;
+		goto fail_put_clk;
 	}
 #endif
 
@@ -338,8 +331,6 @@ fail_cpufreq_unregister:
 #ifdef CONFIG_CPU_FREQ
 	cpufreq_unregister_notifier(&priv->freq_transition,
 			CPUFREQ_TRANSITION_NOTIFIER);
-fail_put_disp_clk:
-	clk_put(priv->disp_clk);
 #endif
 
 fail_put_clk:
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
index e863ad0..c00f518 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
@@ -49,7 +49,6 @@
 struct tilcdc_drm_private {
 	void __iomem *mmio;
 
-	struct clk *disp_clk;    /* display dpll */
 	struct clk *clk;         /* functional clock */
 	int rev;                 /* IP revision */
 
-- 
1.9.1

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

[PATCH 03/12] drm/tilcdc: verify fb pitch

[Jyri Sarha]

From: Tomi Valkeinen <tomi.valkeinen@ti.com>

LCDC hardware does not support fb pitch that is different (i.e. larger)
than the screen size. The driver currently does no checks for this, and
the results of too big pitch are are flickering and lower fps.

This issue easily happens when using libdrm's modetest tool with non-32
bpp modes. As modetest always allocated 4 bytes per pixel, it implies a
bigger pitch for 16 or 24 bpp modes.

This patch adds a check to reject pitches the hardware cannot support.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 7b687ae..105f286 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -151,6 +151,22 @@ static void tilcdc_crtc_destroy(struct drm_crtc *crtc)
 	kfree(tilcdc_crtc);
 }
 
+static int tilcdc_verify_fb(struct drm_crtc *crtc, struct drm_framebuffer *fb)
+{
+	struct drm_device *dev = crtc->dev;
+	unsigned int depth, bpp;
+
+	drm_fb_get_bpp_depth(fb->pixel_format, &depth, &bpp);
+
+	if (fb->pitches[0] != crtc->mode.hdisplay * bpp / 8) {
+		dev_err(dev->dev,
+			"Invalid pitch: fb and crtc widths must be the same");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int tilcdc_crtc_page_flip(struct drm_crtc *crtc,
 		struct drm_framebuffer *fb,
 		struct drm_pending_vblank_event *event,
@@ -158,6 +174,11 @@ static int tilcdc_crtc_page_flip(struct drm_crtc *crtc,
 {
 	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
+	int r;
+
+	r = tilcdc_verify_fb(crtc, fb);
+	if (r)
+		return r;
 
 	if (tilcdc_crtc->event) {
 		dev_err(dev->dev, "already pending page flip!\n");
@@ -272,6 +293,10 @@ static int tilcdc_crtc_mode_set(struct drm_crtc *crtc,
 	if (WARN_ON(!info))
 		return -EINVAL;
 
+	ret = tilcdc_verify_fb(crtc, crtc->primary->fb);
+	if (ret)
+		return ret;
+
 	pm_runtime_get_sync(dev->dev);
 
 	/* Configure the Burst Size and fifo threshold of DMA: */
@@ -431,6 +456,12 @@ static int tilcdc_crtc_mode_set(struct drm_crtc *crtc,
 static int tilcdc_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 		struct drm_framebuffer *old_fb)
 {
+	int r;
+
+	r = tilcdc_verify_fb(crtc, crtc->primary->fb);
+	if (r)
+		return r;
+
 	update_scanout(crtc);
 	return 0;
 }
-- 
1.9.1

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

[PATCH 04/12] drm/tilcdc: adopt pinctrl support

[Jyri Sarha]

From: Dave Gerlach <d-gerlach@ti.com>

Update tilcdc driver to set the state of the pins to:
- "default on resume
- "sleep" on suspend

By optionally putting the pins into sleep state in the suspend callback
we can accomplish two things.
- minimize current leakage from pins and thus save power,
- prevent the IP from driving pins output in an uncontrolled manner,
which may happen if the power domain drops the domain regulator.

Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 2b2780e..146399a 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -18,6 +18,7 @@
 /* LCDC DRM driver, based on da8xx-fb */
 
 #include <linux/component.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/suspend.h>
 
 #include "tilcdc_drv.h"
@@ -601,6 +602,9 @@ static int tilcdc_pm_suspend(struct device *dev)
 		if (registers[i].save && (priv->rev >= registers[i].rev))
 			priv->saved_register[n++] = tilcdc_read(ddev, registers[i].reg);
 
+	/* Select sleep pin state */
+	pinctrl_pm_select_sleep_state(dev);
+
 	return 0;
 }
 
@@ -610,6 +614,9 @@ static int tilcdc_pm_resume(struct device *dev)
 	struct tilcdc_drm_private *priv = ddev->dev_private;
 	unsigned i, n = 0;
 
+	/* Select default pin state */
+	pinctrl_pm_select_default_state(dev);
+
 	/* Restore register state: */
 	for (i = 0; i < ARRAY_SIZE(registers); i++)
 		if (registers[i].save && (priv->rev >= registers[i].rev))
-- 
1.9.1

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

[PATCH 05/12] drm/tilcdc: fix kernel panic on suspend when no hdmi monitor connected

[Jyri Sarha]

From: Darren Etheridge <detheridge@ti.com>

On BeagleBone Black if no HDMI monitor is connected and suspend
is requested a kernel panic will result:

root@am335x-evm:~# echo mem > /sys/power/state
[ 65.548710] PM: Syncing filesystems ... done.
[ 65.631311] Freezing user space processes ... (elapsed 0.006 seconds) done.
[ 65.648619] Freezing remaining freezable tasks ... (elapsed 0.005 seconds) done.
[ 65.833500] Unhandled fault: external abort on non-linefetch (0x1028) at 0xfa30e004
[ 65.841692] Internal error: : 1028 [#1] SMP ARM
  <snip>
[ 66.105287] [<c03765f0>] (platform_pm_suspend) from [<c037b6d4>] (dpm_run_callback+0x34/0x70)
[ 66.114370] [<c037b6d4>] (dpm_run_callback) from [<c037ba84>] (__device_suspend+0x10c/0x2f4)
[ 66.123357] [<c037ba84>] (__device_suspend) from [<c037d004>] (dpm_suspend+0x58/0x218)
[ 66.131796] [<c037d004>] (dpm_suspend) from [<c008d948>] (suspend_devices_and_enter+0x9c/0x3c0)
[ 66.141055] [<c008d948>] (suspend_devices_and_enter) from [<c008de7c>] (pm_suspend+0x210/0x24c)
[ 66.150312] [<c008de7c>] (pm_suspend) from [<c008cabc>] (state_store+0x68/0xb8)
[ 66.158103] [<c008cabc>] (state_store) from [<c02e9654>] (kobj_attr_store+0x14/0x20)
[ 66.166355] [<c02e9654>] (kobj_attr_store) from [<c0185c70>] (sysfs_kf_write+0x4c/0x50)
[ 66.174883] [<c0185c70>] (sysfs_kf_write) from [<c018926c>] (kernfs_fop_write+0xb4/0x150)
[ 66.183598] [<c018926c>] (kernfs_fop_write) from [<c0122638>] (vfs_write+0xa8/0x180)
[ 66.191846] [<c0122638>] (vfs_write) from [<c01229f8>] (SyS_write+0x40/0x8c)
[ 66.199365] [<c01229f8>] (SyS_write) from [<c000e580>] (ret_fast_syscall+0x0/0x48)
[ 66.207426] Code: e595c210 e5932000 e59cc000 e08c2002 (e592c000)

This is because the lcdc module is not enabled when no monitor is detected
to save power.  However the suspend handler just blindly tries to save the
lcdc state by copying out the pertinent registers. However module is off
so no good things happen when you try and access it.

This patch only saves off the registers if the module is enabled, and
then only restores the registers on resume if they were saved off during
suspend.

Signed-off-by: Darren Etheridge <detheridge@ti.com>
Tested-by: Dave Gerlach <d-gerlach@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c | 23 +++++++++++++++++------
 drivers/gpu/drm/tilcdc/tilcdc_drv.h |  1 +
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 146399a..083b8d4 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -597,13 +597,20 @@ static int tilcdc_pm_suspend(struct device *dev)
 
 	drm_kms_helper_poll_disable(ddev);
 
+	/* Select sleep pin state */
+	pinctrl_pm_select_sleep_state(dev);
+
+	if (pm_runtime_suspended(dev)) {
+		priv->ctx_valid = false;
+		return 0;
+	}
+
 	/* Save register state: */
 	for (i = 0; i < ARRAY_SIZE(registers); i++)
 		if (registers[i].save && (priv->rev >= registers[i].rev))
 			priv->saved_register[n++] = tilcdc_read(ddev, registers[i].reg);
 
-	/* Select sleep pin state */
-	pinctrl_pm_select_sleep_state(dev);
+	priv->ctx_valid = true;
 
 	return 0;
 }
@@ -617,10 +624,14 @@ static int tilcdc_pm_resume(struct device *dev)
 	/* Select default pin state */
 	pinctrl_pm_select_default_state(dev);
 
-	/* Restore register state: */
-	for (i = 0; i < ARRAY_SIZE(registers); i++)
-		if (registers[i].save && (priv->rev >= registers[i].rev))
-			tilcdc_write(ddev, registers[i].reg, priv->saved_register[n++]);
+	if (priv->ctx_valid == true) {
+		/* Restore register state: */
+		for (i = 0; i < ARRAY_SIZE(registers); i++)
+			if (registers[i].save &&
+			    (priv->rev >= registers[i].rev))
+				tilcdc_write(ddev, registers[i].reg,
+					     priv->saved_register[n++]);
+	}
 
 	drm_kms_helper_poll_enable(ddev);
 
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
index c00f518..7d214cc 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
@@ -67,6 +67,7 @@ struct tilcdc_drm_private {
 
 	/* register contents saved across suspend/resume: */
 	u32 saved_register[12];
+	bool ctx_valid;
 
 #ifdef CONFIG_CPU_FREQ
 	struct notifier_block freq_transition;
-- 
1.9.1

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

[PATCH 06/12] drm/tilcdc: make frame_done interrupt active at all times

[Jyri Sarha]

From: Darren Etheridge <detheridge@ti.com>

The frame_done interrupt was only being enabled when the vsync
interrupts were being enabled by DRM.  However the frame_done is
used to determine if the LCD controller has successfully completed
the raster_enable, raster_disable commands and the vsync interrupts
are not always enabled during these operations.

Signed-off-by: Darren Etheridge <detheridge@ti.com>
Tested-by: Dave Gerlach <d-gerlach@ti.com>
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 083b8d4..b5563be 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -383,7 +383,9 @@ static int tilcdc_irq_postinstall(struct drm_device *dev)
 	if (priv->rev == 1)
 		tilcdc_set(dev, LCDC_RASTER_CTRL_REG, LCDC_V1_UNDERFLOW_INT_ENA);
 	else
-		tilcdc_set(dev, LCDC_INT_ENABLE_SET_REG, LCDC_V2_UNDERFLOW_INT_ENA);
+		tilcdc_set(dev, LCDC_INT_ENABLE_SET_REG,
+			   LCDC_V2_UNDERFLOW_INT_ENA |
+			   LCDC_FRAME_DONE);
 
 	return 0;
 }
@@ -417,7 +419,7 @@ static void enable_vblank(struct drm_device *dev, bool enable)
 	} else {
 		reg = LCDC_INT_ENABLE_SET_REG;
 		mask = LCDC_V2_END_OF_FRAME0_INT_ENA |
-			LCDC_V2_END_OF_FRAME1_INT_ENA | LCDC_FRAME_DONE;
+			LCDC_V2_END_OF_FRAME1_INT_ENA;
 	}
 
 	if (enable)
-- 
1.9.1

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

[PATCH 07/12] drm/tilcdc: disable the lcd controller/dma engine when suspend invoked

[Jyri Sarha]

From: Darren Etheridge <detheridge@ti.com>

The LCD controller must be deactivated and all DMA transactions stopped
when the suspend power state is entered otherwise the PRCM causes the L3
bus to get stuck in transition state.

This commit forces the lcdc to be shut down and waits for all pending DMA
transactions to complete as part of the suspend handler for this driver.

Signed-off-by: Darren Etheridge <detheridge@ti.com>
Tested-by: Dave Gerlach <d-gerlach@ti.com>
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 3 +--
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  | 3 +++
 drivers/gpu/drm/tilcdc/tilcdc_drv.h  | 1 +
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 105f286..cc45818 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -138,7 +138,6 @@ static void stop(struct drm_crtc *crtc)
 	tilcdc_clear(dev, LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE);
 }
 
-static void tilcdc_crtc_dpms(struct drm_crtc *crtc, int mode);
 static void tilcdc_crtc_destroy(struct drm_crtc *crtc)
 {
 	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
@@ -192,7 +191,7 @@ static int tilcdc_crtc_page_flip(struct drm_crtc *crtc,
 	return 0;
 }
 
-static void tilcdc_crtc_dpms(struct drm_crtc *crtc, int mode)
+void tilcdc_crtc_dpms(struct drm_crtc *crtc, int mode)
 {
 	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index b5563be..f640b37 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -607,6 +607,9 @@ static int tilcdc_pm_suspend(struct device *dev)
 		return 0;
 	}
 
+	/* Disable the LCDC controller, to avoid locking up the PRCM */
+	tilcdc_crtc_dpms(priv->crtc, DRM_MODE_DPMS_OFF);
+
 	/* Save register state: */
 	for (i = 0; i < ARRAY_SIZE(registers); i++)
 		if (registers[i].save && (priv->rev >= registers[i].rev))
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
index 7d214cc..77c600d 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
@@ -172,5 +172,6 @@ void tilcdc_crtc_set_simulate_vesa_sync(struct drm_crtc *crtc,
 					bool simulate_vesa_sync);
 int tilcdc_crtc_mode_valid(struct drm_crtc *crtc, struct drm_display_mode *mode);
 int tilcdc_crtc_max_width(struct drm_crtc *crtc);
+void tilcdc_crtc_dpms(struct drm_crtc *crtc, int mode);
 
 #endif /* __TILCDC_DRV_H__ */
-- 
1.9.1

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

[PATCH 08/12] drm/tilcdc: fix the ping-pong dma tearing issue seen on buffer flipping

[Jyri Sarha]

From: Darren Etheridge <detheridge@ti.com>

Update the DMA pointers during the pan display ioctl. Borrowed from
da8xx_fb.c update the scanout buffers immediately so the DMA ping/pong
doesn't end up out of sync with what we really want it to DMA,
otherwise part of the screen ends up tearing during rapid flip
operations.

Ported from commit deb95c6c958f ("video: da8xx-fb: fix flicker due to
1 frame delay in updated frame")

Signed-off-by: Darren Etheridge <detheridge@ti.com>
[Rewrapped description and fixed commit reference]
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index cc45818..50384fa 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -31,6 +31,8 @@ struct tilcdc_crtc {
 	int dpms;
 	wait_queue_head_t frame_done_wq;
 	bool frame_done;
+	spinlock_t irq_lock;
+	int dma_completed_channel;
 
 	/* fb currently set to scanout 0/1: */
 	struct drm_framebuffer *scanout[2];
@@ -102,10 +104,23 @@ static void update_scanout(struct drm_crtc *crtc)
 			(crtc->mode.vdisplay * fb->pitches[0]);
 
 	if (tilcdc_crtc->dpms == DRM_MODE_DPMS_ON) {
-		/* already enabled, so just mark the frames that need
-		 * updating and they will be updated on vblank:
+		/*
+		 * already enabled, so just mark the frames that need
+		 * updating and they will be updated on vblank
+		 * and update the inactive DMA channel immediately
+		 * to avoid any tearing due to the DMA already starting
+		 * on the pending dma buffer when we hit the vblank IRQ
 		 */
-		tilcdc_crtc->dirty |= LCDC_END_OF_FRAME0 | LCDC_END_OF_FRAME1;
+		if (tilcdc_crtc->dma_completed_channel == 0) {
+			tilcdc_crtc->dirty |= LCDC_END_OF_FRAME1;
+			set_scanout(crtc, 0);
+		}
+
+		if (tilcdc_crtc->dma_completed_channel == 1) {
+			tilcdc_crtc->dirty |= LCDC_END_OF_FRAME0;
+			set_scanout(crtc, 1);
+		}
+
 		drm_vblank_get(dev, 0);
 	} else {
 		/* not enabled yet, so update registers immediately: */
@@ -647,6 +662,7 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
 	struct drm_device *dev = crtc->dev;
 	struct tilcdc_drm_private *priv = dev->dev_private;
 	uint32_t stat = tilcdc_read_irqstatus(dev);
+	unsigned long irq_flags;
 
 	if ((stat & LCDC_SYNC_LOST) && (stat & LCDC_FIFO_UNDERFLOW)) {
 		stop(crtc);
@@ -662,11 +678,19 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
 
 		tilcdc_clear_irqstatus(dev, stat);
 
-		if (dirty & LCDC_END_OF_FRAME0)
+		spin_lock_irqsave(&tilcdc_crtc->irq_lock, irq_flags);
+
+		if (dirty & LCDC_END_OF_FRAME0) {
 			set_scanout(crtc, 0);
+			tilcdc_crtc->dma_completed_channel = 0;
+		}
 
-		if (dirty & LCDC_END_OF_FRAME1)
+		if (dirty & LCDC_END_OF_FRAME1) {
 			set_scanout(crtc, 1);
+			tilcdc_crtc->dma_completed_channel = 1;
+		}
+
+		spin_unlock_irqrestore(&tilcdc_crtc->irq_lock, irq_flags);
 
 		drm_handle_vblank(dev, 0);
 
@@ -732,6 +756,8 @@ struct drm_crtc *tilcdc_crtc_create(struct drm_device *dev)
 	drm_flip_work_init(&tilcdc_crtc->unref_work,
 			"unref", unref_worker);
 
+	spin_lock_init(&tilcdc_crtc->irq_lock);
+
 	ret = drm_crtc_init(dev, crtc, &tilcdc_crtc_funcs);
 	if (ret < 0)
 		goto fail;
-- 
1.9.1

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

[PATCH 09/12] drm/tilcdc: correct the dmachannel tracking logic

[Jyri Sarha]

From: Darren Etheridge <detheridge@ti.com>

dma_channel_completed should be reset to channel 0 when the start
function is called and the dma channel in use should be tracked even
when no flip is pending.

Signed-off-by: Darren Etheridge <detheridge@ti.com>
[Rewrapped description]
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 50384fa..720a43a 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -133,6 +133,7 @@ static void start(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);
 
 	if (priv->rev == 2) {
 		tilcdc_set(dev, LCDC_CLK_RESET_REG, LCDC_CLK_MAIN_RESET);
@@ -141,6 +142,8 @@ static void start(struct drm_crtc *crtc)
 		msleep(1);
 	}
 
+	tilcdc_crtc->dma_completed_channel = 0;
+
 	tilcdc_set(dev, LCDC_DMA_CTRL_REG, LCDC_DUAL_FRAME_BUFFER_ENABLE);
 	tilcdc_set(dev, LCDC_RASTER_CTRL_REG, LCDC_PALETTE_LOAD_MODE(DATA_ONLY));
 	tilcdc_set(dev, LCDC_RASTER_CTRL_REG, LCDC_RASTER_ENABLE);
@@ -680,15 +683,17 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
 
 		spin_lock_irqsave(&tilcdc_crtc->irq_lock, irq_flags);
 
-		if (dirty & LCDC_END_OF_FRAME0) {
-			set_scanout(crtc, 0);
+		if (stat & LCDC_END_OF_FRAME0)
 			tilcdc_crtc->dma_completed_channel = 0;
-		}
 
-		if (dirty & LCDC_END_OF_FRAME1) {
-			set_scanout(crtc, 1);
+		if (stat & LCDC_END_OF_FRAME1)
 			tilcdc_crtc->dma_completed_channel = 1;
-		}
+
+		if (dirty & LCDC_END_OF_FRAME0)
+			set_scanout(crtc, 0);
+
+		if (dirty & LCDC_END_OF_FRAME1)
+			set_scanout(crtc, 1);
 
 		spin_unlock_irqrestore(&tilcdc_crtc->irq_lock, irq_flags);
 
-- 
1.9.1

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

[PATCH 10/12] drm/tilcdc: make frame completion interrupt always enabled

[Jyri Sarha]

From: Darren Etheridge <detheridge@ti.com>

DRM allows vblank handling to be controlled dynamically. Therefore if
vblank interrupts are not needed by anybody they are disabled.
However on tilcdc a ping-pong dma is used and there is no way to find
out from the hardware which dma channel will be used for the next
frame to program the corresponding dma registers with the new
framebuffer address. The only way currently known is to use the vsync
handler to track the channel currently used. This means that disabling
the vsync is a bad idea because it is always needed to track the dma
channel.

This patch removes the enable/disable_vblank functions, moves the
enabling of the FRAME0/FRAME1_DONE interrupts into the
post_install_interrupt handler so they are always active and removes
the calls to drm_vblank_get, drm_vblank_put.

Signed-off-by: Darren Etheridge <detheridge@ti.com>
[Rewrapped descrription]
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c |  6 ------
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  | 26 +++-----------------------
 2 files changed, 3 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 720a43a..8543d6b 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -89,7 +89,6 @@ static void set_scanout(struct drm_crtc *crtc, int n)
 static void update_scanout(struct drm_crtc *crtc)
 {
 	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
-	struct drm_device *dev = crtc->dev;
 	struct drm_framebuffer *fb = crtc->primary->fb;
 	struct drm_gem_cma_object *gem;
 	unsigned int depth, bpp;
@@ -120,8 +119,6 @@ static void update_scanout(struct drm_crtc *crtc)
 			tilcdc_crtc->dirty |= LCDC_END_OF_FRAME0;
 			set_scanout(crtc, 1);
 		}
-
-		drm_vblank_get(dev, 0);
 	} else {
 		/* not enabled yet, so update registers immediately: */
 		set_scanout(crtc, 0);
@@ -706,8 +703,6 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
 			drm_send_vblank_event(dev, 0, event);
 		spin_unlock_irqrestore(&dev->event_lock, flags);
 
-		if (dirty && !tilcdc_crtc->dirty)
-			drm_vblank_put(dev, 0);
 	}
 
 	if (priv->rev == 2) {
@@ -736,7 +731,6 @@ void tilcdc_crtc_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file)
 	if (event && event->base.file_priv == file) {
 		tilcdc_crtc->event = NULL;
 		event->base.destroy(&event->base);
-		drm_vblank_put(dev, 0);
 	}
 	spin_unlock_irqrestore(&dev->event_lock, flags);
 }
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index f640b37..2b6d2dc 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -385,6 +385,8 @@ static int tilcdc_irq_postinstall(struct drm_device *dev)
 	else
 		tilcdc_set(dev, LCDC_INT_ENABLE_SET_REG,
 			   LCDC_V2_UNDERFLOW_INT_ENA |
+			   LCDC_V2_END_OF_FRAME0_INT_ENA |
+			   LCDC_V2_END_OF_FRAME1_INT_ENA |
 			   LCDC_FRAME_DONE);
 
 	return 0;
@@ -405,38 +407,16 @@ static void tilcdc_irq_uninstall(struct drm_device *dev)
 			LCDC_V2_END_OF_FRAME0_INT_ENA | LCDC_V2_END_OF_FRAME1_INT_ENA |
 			LCDC_FRAME_DONE);
 	}
-
-}
-
-static void enable_vblank(struct drm_device *dev, bool enable)
-{
-	struct tilcdc_drm_private *priv = dev->dev_private;
-	u32 reg, mask;
-
-	if (priv->rev == 1) {
-		reg = LCDC_DMA_CTRL_REG;
-		mask = LCDC_V1_END_OF_FRAME_INT_ENA;
-	} else {
-		reg = LCDC_INT_ENABLE_SET_REG;
-		mask = LCDC_V2_END_OF_FRAME0_INT_ENA |
-			LCDC_V2_END_OF_FRAME1_INT_ENA;
-	}
-
-	if (enable)
-		tilcdc_set(dev, reg, mask);
-	else
-		tilcdc_clear(dev, reg, mask);
 }
 
 static int tilcdc_enable_vblank(struct drm_device *dev, unsigned int pipe)
 {
-	enable_vblank(dev, true);
 	return 0;
 }
 
 static void tilcdc_disable_vblank(struct drm_device *dev, unsigned int pipe)
 {
-	enable_vblank(dev, false);
+	return;
 }
 
 #if defined(CONFIG_DEBUG_FS) || defined(CONFIG_PM_SLEEP)
-- 
1.9.1

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

[PATCH 11/12] drm/tilcdc: Implement dma-buf support for tilcdc

[Jyri Sarha]

There is nothing special about tilcdc HW when the video memory is
concerned. Just using the standard drm helpers for implementation is
enough.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 2b6d2dc..105d938 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -536,7 +536,8 @@ static const struct file_operations fops = {
 };
 
 static struct drm_driver tilcdc_driver = {
-	.driver_features    = DRIVER_HAVE_IRQ | DRIVER_GEM | DRIVER_MODESET,
+	.driver_features    = (DRIVER_HAVE_IRQ | DRIVER_GEM | DRIVER_MODESET |
+			       DRIVER_PRIME),
 	.load               = tilcdc_load,
 	.unload             = tilcdc_unload,
 	.preclose           = tilcdc_preclose,
@@ -554,6 +555,16 @@ static struct drm_driver tilcdc_driver = {
 	.dumb_create        = drm_gem_cma_dumb_create,
 	.dumb_map_offset    = drm_gem_cma_dumb_map_offset,
 	.dumb_destroy       = drm_gem_dumb_destroy,
+
+	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd,
+	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle,
+	.gem_prime_import	= drm_gem_prime_import,
+	.gem_prime_export	= drm_gem_prime_export,
+	.gem_prime_get_sg_table	= drm_gem_cma_prime_get_sg_table,
+	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
+	.gem_prime_vmap		= drm_gem_cma_prime_vmap,
+	.gem_prime_vunmap	= drm_gem_cma_prime_vunmap,
+	.gem_prime_mmap		= drm_gem_cma_prime_mmap,
 #ifdef CONFIG_DEBUG_FS
 	.debugfs_init       = tilcdc_debugfs_init,
 	.debugfs_cleanup    = tilcdc_debugfs_cleanup,
-- 
1.9.1

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

[PATCH 12/12] drm/tilcdc: fix build error when !CONFIG_CPU_FREQ

[Jyri Sarha]

From: Grygorii Strashko <Grygorii.Strashko@linaro.org>

Fix build error when !CONFIG_CPU_FREQ
drivers/gpu/drm/tilcdc/tilcdc_drv.c: In function 'tilcdc_load':
drivers/gpu/drm/tilcdc/tilcdc_drv.c:327:1: error: label 'fail_put_clk' defined but not used [-Werror=unused-label]
 fail_put_clk:
 ^

Signed-off-by: Grygorii Strashko <Grygorii.Strashko@linaro.org>
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 105d938..294e6c5 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -332,9 +332,9 @@ fail_cpufreq_unregister:
 #ifdef CONFIG_CPU_FREQ
 	cpufreq_unregister_notifier(&priv->freq_transition,
 			CPUFREQ_TRANSITION_NOTIFIER);
-#endif
 
 fail_put_clk:
+#endif
 	clk_put(priv->clk);
 
 fail_iounmap:
-- 
1.9.1

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

Re: [PATCH 00/12] drm/tilcdc: fixes and added features from TI tree

[Tomi Valkeinen]

[-- Attachment #1.1: Type: text/plain, Size: 578 bytes --]

Hi,

On 15/12/15 21:03, Jyri Sarha wrote:
> Most of these patches are quite old and they have been used in TI
> linux for some time already. I forward ported the applicaple patches
> from TI tree on top of the latest drm-next, fixed checkpatch issues
> and tested the driver.

I think patches 6-10 can be dropped, as the other not-yet-posted series
will fix those issues properly.

Patch 1 is unclear. We have similar one for omapdrm in the TI kernel,
but I haven't pushed it to mainline as I'm not sure if it's valid. No
other driver seems to need it.

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 03/12] drm/tilcdc: verify fb pitch

[Daniel Vetter]

On Tue, Dec 15, 2015 at 09:03:14PM +0200, Jyri Sarha wrote:
> From: Tomi Valkeinen <tomi.valkeinen@ti.com>
> 
> LCDC hardware does not support fb pitch that is different (i.e. larger)
> than the screen size. The driver currently does no checks for this, and
> the results of too big pitch are are flickering and lower fps.
> 
> This issue easily happens when using libdrm's modetest tool with non-32
> bpp modes. As modetest always allocated 4 bytes per pixel, it implies a
> bigger pitch for 16 or 24 bpp modes.
> 
> This patch adds a check to reject pitches the hardware cannot support.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Signed-off-by: Darren Etheridge <detheridge@ti.com>
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> index 7b687ae..105f286 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> @@ -151,6 +151,22 @@ static void tilcdc_crtc_destroy(struct drm_crtc *crtc)
>  	kfree(tilcdc_crtc);
>  }
>  
> +static int tilcdc_verify_fb(struct drm_crtc *crtc, struct drm_framebuffer *fb)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	unsigned int depth, bpp;
> +
> +	drm_fb_get_bpp_depth(fb->pixel_format, &depth, &bpp);
> +
> +	if (fb->pitches[0] != crtc->mode.hdisplay * bpp / 8) {
> +		dev_err(dev->dev,
> +			"Invalid pitch: fb and crtc widths must be the same");
> +		return -EINVAL;
> +	}
> +
> +	return 0;

This should be done in framebuffer_create instead if tilcdc has this
requirement everywhere. No point in allowing userspace to create an fb you
can't use. Only if you have planes with different limits should this be
checked after fb creation.

Also with atomic you'd only need to place this in one function, even if
you have different per-plane limitations ... hint, hint ;-)

Cheers, Daniel


> +}
> +
>  static int tilcdc_crtc_page_flip(struct drm_crtc *crtc,
>  		struct drm_framebuffer *fb,
>  		struct drm_pending_vblank_event *event,
> @@ -158,6 +174,11 @@ static int tilcdc_crtc_page_flip(struct drm_crtc *crtc,
>  {
>  	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
>  	struct drm_device *dev = crtc->dev;
> +	int r;
> +
> +	r = tilcdc_verify_fb(crtc, fb);
> +	if (r)
> +		return r;
>  
>  	if (tilcdc_crtc->event) {
>  		dev_err(dev->dev, "already pending page flip!\n");
> @@ -272,6 +293,10 @@ static int tilcdc_crtc_mode_set(struct drm_crtc *crtc,
>  	if (WARN_ON(!info))
>  		return -EINVAL;
>  
> +	ret = tilcdc_verify_fb(crtc, crtc->primary->fb);
> +	if (ret)
> +		return ret;
> +
>  	pm_runtime_get_sync(dev->dev);
>  
>  	/* Configure the Burst Size and fifo threshold of DMA: */
> @@ -431,6 +456,12 @@ static int tilcdc_crtc_mode_set(struct drm_crtc *crtc,
>  static int tilcdc_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
>  		struct drm_framebuffer *old_fb)
>  {
> +	int r;
> +
> +	r = tilcdc_verify_fb(crtc, crtc->primary->fb);
> +	if (r)
> +		return r;
> +
>  	update_scanout(crtc);
>  	return 0;
>  }
> -- 
> 1.9.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 03/12] drm/tilcdc: verify fb pitch

[Jyri Sarha]

On 12/16/15 10:37, Daniel Vetter wrote:
> On Tue, Dec 15, 2015 at 09:03:14PM +0200, Jyri Sarha wrote:
>> >From: Tomi Valkeinen<tomi.valkeinen@ti.com>
>> >
>> >LCDC hardware does not support fb pitch that is different (i.e. larger)
>> >than the screen size. The driver currently does no checks for this, and
>> >the results of too big pitch are are flickering and lower fps.
>> >
>> >This issue easily happens when using libdrm's modetest tool with non-32
>> >bpp modes. As modetest always allocated 4 bytes per pixel, it implies a
>> >bigger pitch for 16 or 24 bpp modes.
>> >
>> >This patch adds a check to reject pitches the hardware cannot support.
>> >
>> >Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
>> >Signed-off-by: Darren Etheridge<detheridge@ti.com>
>> >Signed-off-by: Jyri Sarha<jsarha@ti.com>
>> >---
>> >  drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 31 +++++++++++++++++++++++++++++++
>> >  1 file changed, 31 insertions(+)
>> >
>> >diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
>> >index 7b687ae..105f286 100644
>> >--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
>> >+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
>> >@@ -151,6 +151,22 @@ static void tilcdc_crtc_destroy(struct drm_crtc *crtc)
>> >  	kfree(tilcdc_crtc);
>> >  }
>> >
>> >+static int tilcdc_verify_fb(struct drm_crtc *crtc, struct drm_framebuffer *fb)
>> >+{
>> >+	struct drm_device *dev = crtc->dev;
>> >+	unsigned int depth, bpp;
>> >+
>> >+	drm_fb_get_bpp_depth(fb->pixel_format, &depth, &bpp);
>> >+
>> >+	if (fb->pitches[0] != crtc->mode.hdisplay * bpp / 8) {
>> >+		dev_err(dev->dev,
>> >+			"Invalid pitch: fb and crtc widths must be the same");
>> >+		return -EINVAL;
>> >+	}
>> >+
>> >+	return 0;
> This should be done in framebuffer_create instead if tilcdc has this
> requirement everywhere. No point in allowing userspace to create an fb you
> can't use. Only if you have planes with different limits should this be
> checked after fb creation.
>

That would not work because we do not know the mode of the crtc when the 
framebuffer is going to be used.

> Also with atomic you'd only need to place this in one function, even if
> you have different per-plane limitations ... hint, hint;-)

I am working on atomic modeset for tilcdc, but I am still a newbie on 
DRM front so it takes some time :).

Cheers,
Jyri


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

Re: [PATCH 03/12] drm/tilcdc: verify fb pitch

[Daniel Vetter]

On Wed, Feb 10, 2016 at 03:18:01PM +0200, Jyri Sarha wrote:
> On 12/16/15 10:37, Daniel Vetter wrote:
> >On Tue, Dec 15, 2015 at 09:03:14PM +0200, Jyri Sarha wrote:
> >>>From: Tomi Valkeinen<tomi.valkeinen@ti.com>
> >>>
> >>>LCDC hardware does not support fb pitch that is different (i.e. larger)
> >>>than the screen size. The driver currently does no checks for this, and
> >>>the results of too big pitch are are flickering and lower fps.
> >>>
> >>>This issue easily happens when using libdrm's modetest tool with non-32
> >>>bpp modes. As modetest always allocated 4 bytes per pixel, it implies a
> >>>bigger pitch for 16 or 24 bpp modes.
> >>>
> >>>This patch adds a check to reject pitches the hardware cannot support.
> >>>
> >>>Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
> >>>Signed-off-by: Darren Etheridge<detheridge@ti.com>
> >>>Signed-off-by: Jyri Sarha<jsarha@ti.com>
> >>>---
> >>>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 31 +++++++++++++++++++++++++++++++
> >>>  1 file changed, 31 insertions(+)
> >>>
> >>>diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> >>>index 7b687ae..105f286 100644
> >>>--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> >>>+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
> >>>@@ -151,6 +151,22 @@ static void tilcdc_crtc_destroy(struct drm_crtc *crtc)
> >>>  	kfree(tilcdc_crtc);
> >>>  }
> >>>
> >>>+static int tilcdc_verify_fb(struct drm_crtc *crtc, struct drm_framebuffer *fb)
> >>>+{
> >>>+	struct drm_device *dev = crtc->dev;
> >>>+	unsigned int depth, bpp;
> >>>+
> >>>+	drm_fb_get_bpp_depth(fb->pixel_format, &depth, &bpp);
> >>>+
> >>>+	if (fb->pitches[0] != crtc->mode.hdisplay * bpp / 8) {
> >>>+		dev_err(dev->dev,
> >>>+			"Invalid pitch: fb and crtc widths must be the same");
> >>>+		return -EINVAL;
> >>>+	}
> >>>+
> >>>+	return 0;
> >This should be done in framebuffer_create instead if tilcdc has this
> >requirement everywhere. No point in allowing userspace to create an fb you
> >can't use. Only if you have planes with different limits should this be
> >checked after fb creation.
> >
> 
> That would not work because we do not know the mode of the crtc when the
> framebuffer is going to be used.

My apologies, I didn't read your code careful. Let me blame jetlag on this
;-) Makes sense on 2nd look.

> >Also with atomic you'd only need to place this in one function, even if
> >you have different per-plane limitations ... hint, hint;-)
> 
> I am working on atomic modeset for tilcdc, but I am still a newbie on DRM
> front so it takes some time :).

No worries, there's tons of good atomic drivers as examples now.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel