[1/1] drm/panfrost: remove spamming to syslog

Commit Message

Eero Lehtinen June 8, 2021, 5:18 p.m. UTC

This patch removes Panfrost spamming messages to syslog that causes a
poor performance and crashes of the Xfce desktop with a Amlogic S912
TV box. See the old bug in:
https://gitlab.freedesktop.org/mesa/mesa/-/issues/3143

Signed-off-by: Eero Lehtinen <debiangamer2 at gmail.com>

Comments

Robin Murphy June 9, 2021, 10:07 a.m. UTC | #1

On 2021-06-08 18:18, Eero Lehtinen wrote:
> This patch removes Panfrost spamming messages to syslog that causes a
> poor performance and crashes of the Xfce desktop with a Amlogic S912
> TV box. See the old bug in:
> https://gitlab.freedesktop.org/mesa/mesa/-/issues/3143

There is certainly an argument for demoting the shrinker messages to a 
debug feature, if we're happy that our memory management is mature 
enough that memory pressure issues are no longer a common thing to need 
to spot easily (and especially since msm, from whence that was copied, 
has now taken a similar route with commit fdf38426cda6). The rest, 
however, represent genuine error conditions, so hiding them is 
nonsensical. If the GPU is faulting or locked up, 99% of the time it's 
because the userspace driver has done *something* wrong (for incredibly 
many values of "something"), but there's still the possibility of things 
going wrong in the kernel too. Either way, the bugs only have a hope of 
getting fixed if we know they're happening.

Taking the "this is fine" approach of wilfully ignoring significant 
errors because your particular use-case happens to be able to limp along 
in spite of them might be OK as a local hack, but it is not reasonable 
for the mainline kernel.

Thanks,
Robin.

> Signed-off-by: Eero Lehtinen <debiangamer2 at gmail.com>
> 
> --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> @@ -36,8 +36,8 @@
>       ret = readl_relaxed_poll_timeout_atomic(pfdev->iomem + AS_STATUS(as_nr),
>           val, !(val & AS_STATUS_AS_ACTIVE), 10, 1000);
> 
> -    if (ret)
> -        dev_err(pfdev->dev, "AS_ACTIVE bit stuck\n");
> +    //if (ret)
> +    //    dev_err(pfdev->dev, "AS_ACTIVE bit stuck\n");
> 
>       return ret;
>   }
> 
> --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> @@ -292,8 +292,8 @@
>           prot |= IOMMU_NOEXEC;
> 
>       sgt = drm_gem_shmem_get_pages_sgt(obj);
> -    if (WARN_ON(IS_ERR(sgt)))
> -        return PTR_ERR(sgt);
> +    //if (WARN_ON(IS_ERR(sgt)))
> +    //    return PTR_ERR(sgt);
> 
>       mmu_map_sg(pfdev, mapping->mmu, mapping->mmnode.start << PAGE_SHIFT,
>              prot, sgt);
> 
> --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> @@ -660,7 +660,7 @@
>           ret = -1;
>           if ((status & mask) == BIT(as) && (exception_type & 0xF8) == 0xC0)
>               ret = panfrost_mmu_map_fault_addr(pfdev, as, addr);
> -
> +#if 0
>           if (ret)
>               /* terminal fault, print info about the fault */
>               dev_err(pfdev->dev,
> @@ -678,7 +678,7 @@
>                   exception_type, panfrost_exception_name(pfdev, exception_type),
>                   access_type, access_type_name(pfdev, fault_status),
>                   source_id);
> -
> +#endif
>           status &= ~mask;
> 
>           /* If we received new MMU interrupts, process them before returning. */
> 
> --- a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
> @@ -85,8 +85,8 @@
> 
>       mutex_unlock(&pfdev->shrinker_lock);
> 
> -    if (freed > 0)
> -        pr_info_ratelimited("Purging %lu bytes\n", freed << PAGE_SHIFT);
> +    // if (freed > 0)
> +        // pr_info_ratelimited("Purging %lu bytes\n", freed << PAGE_SHIFT);
> 
>       return freed;
>   }
>

Patch

--- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
@@ -36,8 +36,8 @@ 
     ret = readl_relaxed_poll_timeout_atomic(pfdev->iomem + AS_STATUS(as_nr),
         val, !(val & AS_STATUS_AS_ACTIVE), 10, 1000);

-    if (ret)
-        dev_err(pfdev->dev, "AS_ACTIVE bit stuck\n");
+    //if (ret)
+    //    dev_err(pfdev->dev, "AS_ACTIVE bit stuck\n");

     return ret;
 }

--- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
@@ -292,8 +292,8 @@ 
         prot |= IOMMU_NOEXEC;

     sgt = drm_gem_shmem_get_pages_sgt(obj);
-    if (WARN_ON(IS_ERR(sgt)))
-        return PTR_ERR(sgt);
+    //if (WARN_ON(IS_ERR(sgt)))
+    //    return PTR_ERR(sgt);

     mmu_map_sg(pfdev, mapping->mmu, mapping->mmnode.start << PAGE_SHIFT,
            prot, sgt);

--- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
@@ -660,7 +660,7 @@ 
         ret = -1;
         if ((status & mask) == BIT(as) && (exception_type & 0xF8) == 0xC0)
             ret = panfrost_mmu_map_fault_addr(pfdev, as, addr);
-
+#if 0
         if (ret)
             /* terminal fault, print info about the fault */
             dev_err(pfdev->dev,
@@ -678,7 +678,7 @@ 
                 exception_type, panfrost_exception_name(pfdev, exception_type),
                 access_type, access_type_name(pfdev, fault_status),
                 source_id);
-
+#endif
         status &= ~mask;

         /* If we received new MMU interrupts, process them before returning. */

--- a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c
@@ -85,8 +85,8 @@ 

     mutex_unlock(&pfdev->shrinker_lock);

-    if (freed > 0)
-        pr_info_ratelimited("Purging %lu bytes\n", freed << PAGE_SHIFT);
+    // if (freed > 0)
+        // pr_info_ratelimited("Purging %lu bytes\n", freed << PAGE_SHIFT);

     return freed;
 }