[PATCH v3 0/3] drm/omap: Workaround for errata i878

[PATCH v3 0/3] drm/omap: Workaround for errata i878

[Peter Ujfalusi]

Hi,

Changes since v2:
- Use threaded irq when the i878 workaround is used to avoid unlikely system
  freeze: dma_sync_wait() have 5 second timeout
- Use mutex instead of spinlock as wa_lock
- use the dmaengine_prep_dma_memcpy() wrapper
- do not explicitly call dma_async_issue_pending() as it is done as part of
  dma_sync_wait()
- Use define for the DMM register size (4 bytes)
- Cleanup patch for the remove path: no need to check if the irq is valid. The
  driver would not probe w/o valid interrupt.

Changes since v1:
- rebased on drm-next
- comments for the v1 (https://patchwork.kernel.org/patch/8358741/) addressed
 - u32 -> dma_addr_t when applicable
 - additional wmb()/rmb() added to make sure we have correct behavior

Errata i878 says that MPU should not be used to access RAM and DMM at
the same time. As it's not possible to prevent MPU accessing RAM, we
need to access DMM via a proxy.

Regards,
Peter
---
Peter Ujfalusi (1):
  drm/omap: dmm_tiler: No need to check if irq is valid in
    omap_dmm_remove

Tomi Valkeinen (2):
  dt-bindings: arm: omap: dmm: Document new compatible for DRA7xx family
  drm/omap: partial workaround for DRA7xx DMM errata i878

 .../devicetree/bindings/arm/omap/dmm.txt      |   3 +-
 drivers/gpu/drm/omapdrm/omap_dmm_priv.h       |   8 +
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c      | 164 +++++++++++++++++-
 3 files changed, 168 insertions(+), 7 deletions(-)

-- 
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

[PATCH v3 1/3] drm/omap: dmm_tiler: No need to check if irq is valid in omap_dmm_remove

[Peter Ujfalusi]

The driver probe would fail if the irq is not available.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index e84871e74615..8671d06c0eb4 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -632,8 +632,7 @@ static int omap_dmm_remove(struct platform_device *dev)
 		if (omap_dmm->dummy_page)
 			__free_page(omap_dmm->dummy_page);
 
-		if (omap_dmm->irq > 0)
-			free_irq(omap_dmm->irq, omap_dmm);
+		free_irq(omap_dmm->irq, omap_dmm);
 
 		iounmap(omap_dmm->base);
 		kfree(omap_dmm);
-- 
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

[PATCH v3 2/3] dt-bindings: arm: omap: dmm: Document new compatible for DRA7xx family

[Peter Ujfalusi]

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

Define unique compatible string for the DMM in DRA7xx family.

The new compatible can be used to apply DRA7xx specific workarounds for
ERRATAs, like i878 (MPU Lockup with concurrent DMM and EMIF accesses)

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/devicetree/bindings/arm/omap/dmm.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/arm/omap/dmm.txt b/Documentation/devicetree/bindings/arm/omap/dmm.txt
index 8bd6d0a238a8..bbbe7cdba30c 100644
--- a/Documentation/devicetree/bindings/arm/omap/dmm.txt
+++ b/Documentation/devicetree/bindings/arm/omap/dmm.txt
@@ -8,7 +8,8 @@ translation for initiators which need contiguous dma bus addresses.
 
 Required properties:
 - compatible:	Should contain "ti,omap4-dmm" for OMAP4 family
-		Should contain "ti,omap5-dmm" for OMAP5 and DRA7x family
+		Should contain "ti,omap5-dmm" for OMAP5 family
+		Should contain "ti,dra7-dmm" for DRA7xx family
 - reg:		Contains DMM register address range (base address and length)
 - interrupts:	Should contain an interrupt-specifier for DMM_IRQ.
 - ti,hwmods:	Name of the hwmod associated to DMM, which is typically "dmm"
-- 
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

Re: [PATCH v3 2/3] dt-bindings: arm: omap: dmm: Document new compatible for DRA7xx family

[Tomi Valkeinen]

On 10/04/18 14:33, Peter Ujfalusi wrote:
> From: Tomi Valkeinen <tomi.valkeinen@ti.com>
> 
> Define unique compatible string for the DMM in DRA7xx family.
> 
> The new compatible can be used to apply DRA7xx specific workarounds for
> ERRATAs, like i878 (MPU Lockup with concurrent DMM and EMIF accesses)
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  Documentation/devicetree/bindings/arm/omap/dmm.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/omap/dmm.txt b/Documentation/devicetree/bindings/arm/omap/dmm.txt
> index 8bd6d0a238a8..bbbe7cdba30c 100644
> --- a/Documentation/devicetree/bindings/arm/omap/dmm.txt
> +++ b/Documentation/devicetree/bindings/arm/omap/dmm.txt
> @@ -8,7 +8,8 @@ translation for initiators which need contiguous dma bus addresses.
>  
>  Required properties:
>  - compatible:	Should contain "ti,omap4-dmm" for OMAP4 family
> -		Should contain "ti,omap5-dmm" for OMAP5 and DRA7x family
> +		Should contain "ti,omap5-dmm" for OMAP5 family
> +		Should contain "ti,dra7-dmm" for DRA7xx family
>  - reg:		Contains DMM register address range (base address and length)
>  - interrupts:	Should contain an interrupt-specifier for DMM_IRQ.
>  - ti,hwmods:	Name of the hwmod associated to DMM, which is typically "dmm"
> 

Thinking about this now, wouldn't it be better to add a DT property to
enable the work-around? Or we could use soc_device_match() to figure out
that this is a DRA7 SoC.

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 3/3] drm/omap: partial workaround for DRA7xx DMM errata i878

[Peter Ujfalusi]

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

Errata i878 says that MPU should not be used to access RAM and DMM at
the same time. As it's not possible to prevent MPU accessing RAM, we
need to access DMM via a proxy.

This patch changes:
- DMM driver to access DMM registers via sDMA. Instead of doing a normal
readl/writel call to read/write a register, we use sDMA to copy 4 bytes
from/to the DMM registers.
- When the i878 workaround is needed we use threaded irq. It is not a good
practice to busy loop for completion of the DMA register access in the
interrupt handler. The DMA transfer should not take long time to complete,
but if something prevents the transfer to be completed we might end up
waiting for 5 seconds.

This patch provides only a partial workaround for i878, as not only DMM
register reads/writes are affected, but also accesses to the DMM mapped
buffers (framebuffers, usually).

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_dmm_priv.h  |   8 ++
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 161 ++++++++++++++++++++++-
 2 files changed, 165 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_priv.h b/drivers/gpu/drm/omapdrm/omap_dmm_priv.h
index c2785cc98dc9..a0164652db1e 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_priv.h
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_priv.h
@@ -155,10 +155,12 @@ struct refill_engine {
 
 struct dmm_platform_data {
 	u32 cpu_cache_flags;
+	bool errata_i878_wa;
 };
 
 struct dmm {
 	struct device *dev;
+	dma_addr_t phys_base;
 	void __iomem *base;
 	int irq;
 
@@ -189,6 +191,12 @@ struct dmm {
 	struct list_head alloc_head;
 
 	const struct dmm_platform_data *plat_data;
+
+	bool dmm_workaround;
+	struct mutex wa_lock;
+	u32 *wa_dma_data;
+	dma_addr_t wa_dma_handle;
+	struct dma_chan *wa_dma_chan;
 };
 
 #endif
diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index 8671d06c0eb4..fad55f2faa47 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -18,12 +18,14 @@
 #include <linux/completion.h>
 #include <linux/delay.h>
 #include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
 #include <linux/errno.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/list.h>
 #include <linux/mm.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/platform_device.h> /* platform_device() */
 #include <linux/sched.h>
 #include <linux/seq_file.h>
@@ -70,6 +72,7 @@ static const struct {
 	[TILFMT_PAGE]  = GEOM(SLOT_WIDTH_BITS, SLOT_HEIGHT_BITS, 1),
 };
 
+#define DMM_REG_SIZE	4
 
 /* lookup table for registers w/ per-engine instances */
 static const u32 reg[][4] = {
@@ -79,14 +82,135 @@ static const u32 reg[][4] = {
 			DMM_PAT_DESCR__2, DMM_PAT_DESCR__3},
 };
 
+static int dmm_dma_copy(struct dmm *dmm, dma_addr_t src, dma_addr_t dst)
+{
+	struct dma_async_tx_descriptor *tx;
+	enum dma_status status;
+	dma_cookie_t cookie;
+
+	tx = dmaengine_prep_dma_memcpy(dmm->wa_dma_chan, dst, src, 4, 0);
+	if (!tx) {
+		dev_err(dmm->dev, "Failed to prepare DMA memcpy\n");
+		return -EIO;
+	}
+
+	cookie = tx->tx_submit(tx);
+	if (dma_submit_error(cookie)) {
+		dev_err(dmm->dev, "Failed to do DMA tx_submit\n");
+		return -EIO;
+	}
+
+	status = dma_sync_wait(dmm->wa_dma_chan, cookie);
+	if (status != DMA_COMPLETE)
+		dev_err(dmm->dev, "i878 wa DMA copy failure\n");
+
+	dmaengine_terminate_all(dmm->wa_dma_chan);
+	return 0;
+}
+
+static u32 dmm_read_wa(struct dmm *dmm, u32 reg)
+{
+	dma_addr_t src, dst;
+	int r;
+
+	src = dmm->phys_base + reg;
+	dst = dmm->wa_dma_handle;
+
+	r = dmm_dma_copy(dmm, src, dst);
+	if (r) {
+		dev_err(dmm->dev, "sDMA read transfer timeout\n");
+		return readl(dmm->base + reg);
+	}
+
+	/*
+	 * As per i878 workaround, the DMA is used to access the DMM registers.
+	 * Make sure that the readl is not moved by the compiler or the CPU
+	 * earlier than the DMA finished writing the value to memory.
+	 */
+	rmb();
+	return readl(dmm->wa_dma_data);
+}
+
+static void dmm_write_wa(struct dmm *dmm, u32 val, u32 reg)
+{
+	dma_addr_t src, dst;
+	int r;
+
+	writel(val, dmm->wa_dma_data);
+	/*
+	 * As per i878 workaround, the DMA is used to access the DMM registers.
+	 * Make sure that the writel is not moved by the compiler or the CPU, so
+	 * the data will be in place before we start the DMA to do the actual
+	 * register write.
+	 */
+	wmb();
+
+	src = dmm->wa_dma_handle;
+	dst = dmm->phys_base + reg;
+
+	r = dmm_dma_copy(dmm, src, dst);
+	if (r) {
+		dev_err(dmm->dev, "sDMA write transfer timeout\n");
+		writel(val, dmm->base + reg);
+	}
+}
+
 static u32 dmm_read(struct dmm *dmm, u32 reg)
 {
-	return readl(dmm->base + reg);
+	if (dmm->dmm_workaround) {
+		u32 v;
+
+		mutex_lock(&dmm->wa_lock);
+		v = dmm_read_wa(dmm, reg);
+		mutex_unlock(&dmm->wa_lock);
+
+		return v;
+	} else {
+		return readl(dmm->base + reg);
+	}
 }
 
 static void dmm_write(struct dmm *dmm, u32 val, u32 reg)
 {
-	writel(val, dmm->base + reg);
+	if (dmm->dmm_workaround) {
+		mutex_lock(&dmm->wa_lock);
+		dmm_write_wa(dmm, val, reg);
+		mutex_unlock(&dmm->wa_lock);
+	} else {
+		writel(val, dmm->base + reg);
+	}
+}
+
+static int dmm_workaround_init(struct dmm *dmm)
+{
+	dma_cap_mask_t mask;
+
+	mutex_init(&dmm->wa_lock);
+
+	dmm->wa_dma_data = dma_alloc_coherent(dmm->dev, DMM_REG_SIZE,
+					      &dmm->wa_dma_handle, GFP_KERNEL);
+	if (!dmm->wa_dma_data)
+		return -ENOMEM;
+
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_MEMCPY, mask);
+
+	dmm->wa_dma_chan = dma_request_channel(mask, NULL, NULL);
+	if (!dmm->wa_dma_chan) {
+		dma_free_coherent(dmm->dev, DMM_REG_SIZE, dmm->wa_dma_data,
+				  dmm->wa_dma_handle);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+static void dmm_workaround_uninit(struct dmm *dmm)
+{
+	dma_release_channel(dmm->wa_dma_chan);
+
+	dma_free_coherent(dmm->dev, DMM_REG_SIZE, dmm->wa_dma_data,
+			  dmm->wa_dma_handle);
 }
 
 /* simple allocator to grab next 16 byte aligned memory from txn */
@@ -634,6 +758,9 @@ static int omap_dmm_remove(struct platform_device *dev)
 
 		free_irq(omap_dmm->irq, omap_dmm);
 
+		if (omap_dmm->dmm_workaround)
+			dmm_workaround_uninit(omap_dmm);
+
 		iounmap(omap_dmm->base);
 		kfree(omap_dmm);
 		omap_dmm = NULL;
@@ -679,6 +806,7 @@ static int omap_dmm_probe(struct platform_device *dev)
 		goto fail;
 	}
 
+	omap_dmm->phys_base = mem->start;
 	omap_dmm->base = ioremap(mem->start, SZ_2K);
 
 	if (!omap_dmm->base) {
@@ -694,6 +822,17 @@ static int omap_dmm_probe(struct platform_device *dev)
 
 	omap_dmm->dev = &dev->dev;
 
+	if (omap_dmm->plat_data->errata_i878_wa) {
+		if (!dmm_workaround_init(omap_dmm)) {
+			omap_dmm->dmm_workaround = true;
+			dev_info(&dev->dev,
+				"workaround for errata i878 in use\n");
+		} else {
+			dev_warn(&dev->dev,
+				 "failed to initialize work-around for i878\n");
+		}
+	}
+
 	hwinfo = dmm_read(omap_dmm, DMM_PAT_HWINFO);
 	omap_dmm->num_engines = (hwinfo >> 24) & 0x1F;
 	omap_dmm->num_lut = (hwinfo >> 16) & 0x1F;
@@ -720,8 +859,13 @@ static int omap_dmm_probe(struct platform_device *dev)
 	dmm_write(omap_dmm, 0x88888888, DMM_TILER_OR__0);
 	dmm_write(omap_dmm, 0x88888888, DMM_TILER_OR__1);
 
-	ret = request_irq(omap_dmm->irq, omap_dmm_irq_handler, IRQF_SHARED,
-				"omap_dmm_irq_handler", omap_dmm);
+	if (omap_dmm->dmm_workaround)
+		ret = request_threaded_irq(omap_dmm->irq, NULL,
+					   omap_dmm_irq_handler, IRQF_ONESHOT,
+					   "omap_dmm_irq_handler", omap_dmm);
+	else
+		ret = request_irq(omap_dmm->irq, omap_dmm_irq_handler,
+				IRQF_SHARED, "omap_dmm_irq_handler", omap_dmm);
 
 	if (ret) {
 		dev_err(&dev->dev, "couldn't register IRQ %d, error %d\n",
@@ -1057,6 +1201,11 @@ static const struct dmm_platform_data dmm_omap5_platform_data = {
 	.cpu_cache_flags = OMAP_BO_UNCACHED,
 };
 
+static const struct dmm_platform_data dmm_dra7_platform_data = {
+	.cpu_cache_flags = OMAP_BO_UNCACHED,
+	.errata_i878_wa = true,
+};
+
 static const struct of_device_id dmm_of_match[] = {
 	{
 		.compatible = "ti,omap4-dmm",
@@ -1066,6 +1215,10 @@ static const struct of_device_id dmm_of_match[] = {
 		.compatible = "ti,omap5-dmm",
 		.data = &dmm_omap5_platform_data,
 	},
+	{
+		.compatible = "ti,dra7-dmm",
+		.data = &dmm_dra7_platform_data,
+	},
 	{},
 };
 #endif
-- 
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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