Hi,
the following series adds basic error handling and reporting for the dmm/tiler driver.
With the error handling in place we can catch the problems early to avoid more serious consequences.
Regards, Peter --- Peter Ujfalusi (4): drm/omap: DMM: Fix DMM_IRQSTAT_ERR_MASK definition drm/omap: DMM: In case of error/timeout in wait_status() print the reason drm/omap: DMM: Print information if we received an error interrupt drm/omap: DMM: Check for DMM readiness after successful transaction commit
drivers/gpu/drm/omapdrm/omap_dmm_priv.h | 12 ++++++------ drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 22 ++++++++++++++++++++-- 2 files changed, 26 insertions(+), 8 deletions(-)
The error bit definitions are typoed in DMM_IRQSTAT_ERR_MASK which went unnoticed since the DMM_IRQSTAT_ERR_MASK was not used.
Change the bit definitions to the correct ones.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- drivers/gpu/drm/omapdrm/omap_dmm_priv.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_priv.h b/drivers/gpu/drm/omapdrm/omap_dmm_priv.h index 9f32a83ca507..883762ea0aad 100644 --- a/drivers/gpu/drm/omapdrm/omap_dmm_priv.h +++ b/drivers/gpu/drm/omapdrm/omap_dmm_priv.h @@ -59,12 +59,12 @@ #define DMM_IRQSTAT_ERR_UPD_DATA (1<<6) #define DMM_IRQSTAT_ERR_LUT_MISS (1<<7)
-#define DMM_IRQSTAT_ERR_MASK (DMM_IRQ_STAT_ERR_INV_DSC | \ - DMM_IRQ_STAT_ERR_INV_DATA | \ - DMM_IRQ_STAT_ERR_UPD_AREA | \ - DMM_IRQ_STAT_ERR_UPD_CTRL | \ - DMM_IRQ_STAT_ERR_UPD_DATA | \ - DMM_IRQ_STAT_ERR_LUT_MISS) +#define DMM_IRQSTAT_ERR_MASK (DMM_IRQSTAT_ERR_INV_DSC | \ + DMM_IRQSTAT_ERR_INV_DATA | \ + DMM_IRQSTAT_ERR_UPD_AREA | \ + DMM_IRQSTAT_ERR_UPD_CTRL | \ + DMM_IRQSTAT_ERR_UPD_DATA | \ + DMM_IRQSTAT_ERR_LUT_MISS)
#define DMM_PATSTATUS_READY (1<<0) #define DMM_PATSTATUS_VALID (1<<1)
If the wait_status() fails either because of an error reported in the STATUS register or because of a timeout waiting for the wait_mask, print information which might help diagnose the reason.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c index 1dd3dafc59af..5a18f0ead4d5 100644 --- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c +++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c @@ -121,14 +121,22 @@ static int wait_status(struct refill_engine *engine, uint32_t wait_mask) while (true) { r = dmm_read(dmm, reg[PAT_STATUS][engine->id]); err = r & DMM_PATSTATUS_ERR; - if (err) + if (err) { + dev_err(dmm->dev, + "%s: error (engine%d). PAT_STATUS: 0x%08x\n", + __func__, engine->id, r); return -EFAULT; + }
if ((r & wait_mask) == wait_mask) break;
- if (--i == 0) + if (--i == 0) { + dev_err(dmm->dev, + "%s: timeout (engine%d). PAT_STATUS: 0x%08x\n", + __func__, engine->id, r); return -ETIMEDOUT; + }
udelay(1); }
To help diagnose DMM errors, print out information if any of the error bits are set in the interrupt status register.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c index 5a18f0ead4d5..09d09734117e 100644 --- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c +++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c @@ -166,6 +166,11 @@ static irqreturn_t omap_dmm_irq_handler(int irq, void *arg) dmm_write(dmm, status, DMM_PAT_IRQSTATUS);
for (i = 0; i < dmm->num_engines; i++) { + if (status & DMM_IRQSTAT_ERR_MASK) + dev_err(dmm->dev, + "irq error(engine%d): IRQSTAT 0x%02x\n", + i, status & 0xff); + if (status & DMM_IRQSTAT_LST) { if (dmm->engines[i].async) release_engine(&dmm->engines[i]);
Check the status of the DMM engine after it is reported that the transaction was completed as in rare cases the engine might not reached a working state.
The wait_status() will print information in case the DMM is not reached the expected state and the dmm_txn_commit() will return with an error code to make sure that we are not continuing with a broken setup.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c index 09d09734117e..3bd24a254bd0 100644 --- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c +++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c @@ -311,7 +311,12 @@ static int dmm_txn_commit(struct dmm_txn *txn, bool wait) msecs_to_jiffies(100))) { dev_err(dmm->dev, "timed out waiting for done\n"); ret = -ETIMEDOUT; + goto cleanup; } + + /* Check the engine status before continue */ + ret = wait_status(engine, DMM_PATSTATUS_READY | + DMM_PATSTATUS_VALID | DMM_PATSTATUS_DONE); }
cleanup:
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
On 29/09/17 14:49, Peter Ujfalusi wrote:
Hi,
the following series adds basic error handling and reporting for the dmm/tiler driver.
With the error handling in place we can catch the problems early to avoid more serious consequences.
Regards, Peter
Peter Ujfalusi (4): drm/omap: DMM: Fix DMM_IRQSTAT_ERR_MASK definition drm/omap: DMM: In case of error/timeout in wait_status() print the reason drm/omap: DMM: Print information if we received an error interrupt drm/omap: DMM: Check for DMM readiness after successful transaction commit
drivers/gpu/drm/omapdrm/omap_dmm_priv.h | 12 ++++++------ drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 22 ++++++++++++++++++++-- 2 files changed, 26 insertions(+), 8 deletions(-)
Thanks! Applied for v4.16.
tomi
dri-devel@lists.freedesktop.org
-
Peter Ujfalusi -
Tomi Valkeinen