Re: [PATCH v2] drm/panfrost: Don't try to map on error faults
Acked-by: Alyssa Rosenzweig On Wed, Feb 12, 2020 at 02:22:36PM -0600, Rob Herring wrote: > From: Tomeu Vizoso > > If the exception type isn't a translation fault, don't try to map and > instead go straight to a terminal fault. > > Otherwise, we can get flooded by kernel warnings and further faults. > > Signed-off-by: Tomeu Vizoso > Signed-off-by: Rob Herring > --- > I rewrote this some simplifying the code and somewhat following Steven's > suggested. Still not using defines though. No defines here was good > enough before IMO. > > Only compile tested. > > drivers/gpu/drm/panfrost/panfrost_mmu.c | 44 +++-- > 1 file changed, 19 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c > b/drivers/gpu/drm/panfrost/panfrost_mmu.c > index 763cfca886a7..4f2836bd9215 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c > +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c > @@ -596,33 +596,27 @@ static irqreturn_t panfrost_mmu_irq_handler_thread(int > irq, void *data) > source_id = (fault_status >> 16); > > /* Page fault only */ > - if ((status & mask) == BIT(i)) { > - WARN_ON(exception_type < 0xC1 || exception_type > 0xC4); > - > + ret = -1; > + if ((status & mask) == BIT(i) && (exception_type & 0xF8) == > 0xC0) > ret = panfrost_mmu_map_fault_addr(pfdev, i, addr); > - if (!ret) { > - mmu_write(pfdev, MMU_INT_CLEAR, BIT(i)); > - status &= ~mask; > - continue; > - } > - } > > - /* terminal fault, print info about the fault */ > - dev_err(pfdev->dev, > - "Unhandled Page fault in AS%d at VA 0x%016llX\n" > - "Reason: %s\n" > - "raw fault status: 0x%X\n" > - "decoded fault status: %s\n" > - "exception type 0x%X: %s\n" > - "access type 0x%X: %s\n" > - "source id 0x%X\n", > - i, addr, > - "TODO", > - fault_status, > - (fault_status & (1 << 10) ? "DECODER FAULT" : "SLAVE > FAULT"), > - exception_type, panfrost_exception_name(pfdev, > exception_type), > - access_type, access_type_name(pfdev, fault_status), > - source_id); > + if (ret) > + /* terminal fault, print info about the fault */ > + dev_err(pfdev->dev, > + "Unhandled Page fault in AS%d at VA 0x%016llX\n" > + "Reason: %s\n" > + "raw fault status: 0x%X\n" > + "decoded fault status: %s\n" > + "exception type 0x%X: %s\n" > + "access type 0x%X: %s\n" > + "source id 0x%X\n", > + i, addr, > + "TODO", > + fault_status, > + (fault_status & (1 << 10) ? "DECODER FAULT" : > "SLAVE FAULT"), > + exception_type, panfrost_exception_name(pfdev, > exception_type), > + access_type, access_type_name(pfdev, > fault_status), > + source_id); > > mmu_write(pfdev, MMU_INT_CLEAR, mask); > > -- > 2.20.1 > signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/panfrost: Don't try to map on error faults
On Wed, 12 Feb 2020 at 21:22, Rob Herring wrote: > > From: Tomeu Vizoso > > If the exception type isn't a translation fault, don't try to map and > instead go straight to a terminal fault. > > Otherwise, we can get flooded by kernel warnings and further faults. > > Signed-off-by: Tomeu Vizoso > Signed-off-by: Rob Herring > --- > I rewrote this some simplifying the code and somewhat following Steven's > suggested. Still not using defines though. No defines here was good > enough before IMO. > > Only compile tested. Looks good, I also tested it on RK3399. Reviewed-by: Tomeu Vizoso Thanks, Tomeu > drivers/gpu/drm/panfrost/panfrost_mmu.c | 44 +++-- > 1 file changed, 19 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c > b/drivers/gpu/drm/panfrost/panfrost_mmu.c > index 763cfca886a7..4f2836bd9215 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c > +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c > @@ -596,33 +596,27 @@ static irqreturn_t panfrost_mmu_irq_handler_thread(int > irq, void *data) > source_id = (fault_status >> 16); > > /* Page fault only */ > - if ((status & mask) == BIT(i)) { > - WARN_ON(exception_type < 0xC1 || exception_type > > 0xC4); > - > + ret = -1; > + if ((status & mask) == BIT(i) && (exception_type & 0xF8) == > 0xC0) > ret = panfrost_mmu_map_fault_addr(pfdev, i, addr); > - if (!ret) { > - mmu_write(pfdev, MMU_INT_CLEAR, BIT(i)); > - status &= ~mask; > - continue; > - } > - } > > - /* terminal fault, print info about the fault */ > - dev_err(pfdev->dev, > - "Unhandled Page fault in AS%d at VA 0x%016llX\n" > - "Reason: %s\n" > - "raw fault status: 0x%X\n" > - "decoded fault status: %s\n" > - "exception type 0x%X: %s\n" > - "access type 0x%X: %s\n" > - "source id 0x%X\n", > - i, addr, > - "TODO", > - fault_status, > - (fault_status & (1 << 10) ? "DECODER FAULT" : "SLAVE > FAULT"), > - exception_type, panfrost_exception_name(pfdev, > exception_type), > - access_type, access_type_name(pfdev, fault_status), > - source_id); > + if (ret) > + /* terminal fault, print info about the fault */ > + dev_err(pfdev->dev, > + "Unhandled Page fault in AS%d at VA > 0x%016llX\n" > + "Reason: %s\n" > + "raw fault status: 0x%X\n" > + "decoded fault status: %s\n" > + "exception type 0x%X: %s\n" > + "access type 0x%X: %s\n" > + "source id 0x%X\n", > + i, addr, > + "TODO", > + fault_status, > + (fault_status & (1 << 10) ? "DECODER FAULT" : > "SLAVE FAULT"), > + exception_type, > panfrost_exception_name(pfdev, exception_type), > + access_type, access_type_name(pfdev, > fault_status), > + source_id); > > mmu_write(pfdev, MMU_INT_CLEAR, mask); > > -- > 2.20.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/panfrost: Don't try to map on error faults
On 12/02/2020 20:22, Rob Herring wrote: > From: Tomeu Vizoso > > If the exception type isn't a translation fault, don't try to map and > instead go straight to a terminal fault. > > Otherwise, we can get flooded by kernel warnings and further faults. > > Signed-off-by: Tomeu Vizoso > Signed-off-by: Rob Herring > --- > I rewrote this some simplifying the code and somewhat following Steven's > suggested. Still not using defines though. No defines here was good > enough before IMO. Heresy! It's a good thing you're not writing kbase code - there you (seemingly) need to pick a #define which is as long as possible, but then still wrap the code to avoid the 80 character limit... For example[1]. Although shifting the code right might get you extra bonus points, deep nesting seems to be encouraged! ;) [1] https://gitlab.freedesktop.org/panfrost/mali_kbase/blob/master/driver/product/kernel/drivers/gpu/arm/midgard/backend/gpu/mali_kbase_js_backend.c#L156 > > Only compile tested. I've done some quick testing on a Firefly RK3288. But I don't have any (handy) tests to trigger non-TRANSLATION_FAULT MMU faults. Reviewed-by: Steven Price Thanks, Steve > > drivers/gpu/drm/panfrost/panfrost_mmu.c | 44 +++-- > 1 file changed, 19 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c > b/drivers/gpu/drm/panfrost/panfrost_mmu.c > index 763cfca886a7..4f2836bd9215 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c > +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c > @@ -596,33 +596,27 @@ static irqreturn_t panfrost_mmu_irq_handler_thread(int > irq, void *data) > source_id = (fault_status >> 16); > > /* Page fault only */ > - if ((status & mask) == BIT(i)) { > - WARN_ON(exception_type < 0xC1 || exception_type > 0xC4); > - > + ret = -1; > + if ((status & mask) == BIT(i) && (exception_type & 0xF8) == > 0xC0) > ret = panfrost_mmu_map_fault_addr(pfdev, i, addr); > - if (!ret) { > - mmu_write(pfdev, MMU_INT_CLEAR, BIT(i)); > - status &= ~mask; > - continue; > - } > - } > > - /* terminal fault, print info about the fault */ > - dev_err(pfdev->dev, > - "Unhandled Page fault in AS%d at VA 0x%016llX\n" > - "Reason: %s\n" > - "raw fault status: 0x%X\n" > - "decoded fault status: %s\n" > - "exception type 0x%X: %s\n" > - "access type 0x%X: %s\n" > - "source id 0x%X\n", > - i, addr, > - "TODO", > - fault_status, > - (fault_status & (1 << 10) ? "DECODER FAULT" : "SLAVE > FAULT"), > - exception_type, panfrost_exception_name(pfdev, > exception_type), > - access_type, access_type_name(pfdev, fault_status), > - source_id); > + if (ret) > + /* terminal fault, print info about the fault */ > + dev_err(pfdev->dev, > + "Unhandled Page fault in AS%d at VA 0x%016llX\n" > + "Reason: %s\n" > + "raw fault status: 0x%X\n" > + "decoded fault status: %s\n" > + "exception type 0x%X: %s\n" > + "access type 0x%X: %s\n" > + "source id 0x%X\n", > + i, addr, > + "TODO", > + fault_status, > + (fault_status & (1 << 10) ? "DECODER FAULT" : > "SLAVE FAULT"), > + exception_type, panfrost_exception_name(pfdev, > exception_type), > + access_type, access_type_name(pfdev, > fault_status), > + source_id); > > mmu_write(pfdev, MMU_INT_CLEAR, mask); > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/panfrost: Don't try to map on error faults
From: Tomeu Vizoso If the exception type isn't a translation fault, don't try to map and instead go straight to a terminal fault. Otherwise, we can get flooded by kernel warnings and further faults. Signed-off-by: Tomeu Vizoso Signed-off-by: Rob Herring --- I rewrote this some simplifying the code and somewhat following Steven's suggested. Still not using defines though. No defines here was good enough before IMO. Only compile tested. drivers/gpu/drm/panfrost/panfrost_mmu.c | 44 +++-- 1 file changed, 19 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c index 763cfca886a7..4f2836bd9215 100644 --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c @@ -596,33 +596,27 @@ static irqreturn_t panfrost_mmu_irq_handler_thread(int irq, void *data) source_id = (fault_status >> 16); /* Page fault only */ - if ((status & mask) == BIT(i)) { - WARN_ON(exception_type < 0xC1 || exception_type > 0xC4); - + ret = -1; + if ((status & mask) == BIT(i) && (exception_type & 0xF8) == 0xC0) ret = panfrost_mmu_map_fault_addr(pfdev, i, addr); - if (!ret) { - mmu_write(pfdev, MMU_INT_CLEAR, BIT(i)); - status &= ~mask; - continue; - } - } - /* terminal fault, print info about the fault */ - dev_err(pfdev->dev, - "Unhandled Page fault in AS%d at VA 0x%016llX\n" - "Reason: %s\n" - "raw fault status: 0x%X\n" - "decoded fault status: %s\n" - "exception type 0x%X: %s\n" - "access type 0x%X: %s\n" - "source id 0x%X\n", - i, addr, - "TODO", - fault_status, - (fault_status & (1 << 10) ? "DECODER FAULT" : "SLAVE FAULT"), - exception_type, panfrost_exception_name(pfdev, exception_type), - access_type, access_type_name(pfdev, fault_status), - source_id); + if (ret) + /* terminal fault, print info about the fault */ + dev_err(pfdev->dev, + "Unhandled Page fault in AS%d at VA 0x%016llX\n" + "Reason: %s\n" + "raw fault status: 0x%X\n" + "decoded fault status: %s\n" + "exception type 0x%X: %s\n" + "access type 0x%X: %s\n" + "source id 0x%X\n", + i, addr, + "TODO", + fault_status, + (fault_status & (1 << 10) ? "DECODER FAULT" : "SLAVE FAULT"), + exception_type, panfrost_exception_name(pfdev, exception_type), + access_type, access_type_name(pfdev, fault_status), + source_id); mmu_write(pfdev, MMU_INT_CLEAR, mask); -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel