ÓÚ Fri, 11 Jun 2021 11:10:16 +0100 Steven Price <steven.price@xxxxxxx> дµÀ: > On 10/06/2021 14:06, Chunyou Tang wrote: > > Hi Steven, > > Hi Chunyou, > > For some reason I'm not directly receiving your emails (only via the > list) - can you double check your email configuration? > > >>> The GPU exception fault status register(0x3C),the low 8 bit is the > >>> EXCEPTION_TYPE.We can see the description at P3-78 in spec. > > > > You can see the spec > > <arm_heimdall_technical_reference_manual_100612_0001_00_en.pdf>. > > Thanks - please include that in the commit message - there are many > TRMs (one for each GPU) so without the information about exactly which > specification the page number is pretty useless. Sadly this > documentation isn't public which would be even better but I don't > think there are any public specs for this information. > > >> However this change is correct - panfrost_exception_name() should > >> be taking only the lower 8 bits. Even better though would be to to > >> report the full raw fault information as well as the high bits can > >> contain useful information: > >> > >> dev_warn(pfdev->dev, "GPU Fault 0x%08x (%s) at > >> 0x%016llx\n", fault_status, > >> panfrost_exception_name(pfdev, fault_status & > >> 0xFF), address); > > > > So I change it according to what you said? > > Yes, please send a v2. > > Thanks, > > Steve > > > ÓÚ Thu, 10 Jun 2021 11:41:52 +0100 > > Steven Price <steven.price@xxxxxxx> дµÀ: > > > >> The subject should have the prefix "drm/panfrost" and should > >> mention what the patch is changing (not just the filename). > >> > >> On 09/06/2021 07:38, ChunyouTang wrote: ok > >>> From: tangchunyou <tangchunyou@xxxxxxxxxxxxxxxx> > >>> > >>> The GPU exception fault status register(0x3C),the low 8 bit is the > >>> EXCEPTION_TYPE.We can see the description at P3-78 in spec. > >> > >> Nit: When referring to a spec it's always good to mention the name > >> - I'm not sure which specification you found this in. > >> > >>> > >>> Signed-off-by: tangchunyou <tangchunyou@xxxxxxxxxxxxxxxx> > >>> --- > >>> drivers/gpu/drm/panfrost/panfrost_gpu.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c > >>> b/drivers/gpu/drm/panfrost/panfrost_gpu.c index > >>> 2aae636f1cf5..1fffb6a0b24f 100644 --- > >>> a/drivers/gpu/drm/panfrost/panfrost_gpu.c +++ > >>> b/drivers/gpu/drm/panfrost/panfrost_gpu.c @@ -33,7 +33,7 @@ static > >>> irqreturn_t panfrost_gpu_irq_handler(int irq, void *data) address > >>> |= gpu_read(pfdev, GPU_FAULT_ADDRESS_LO); > >>> dev_warn(pfdev->dev, "GPU Fault 0x%08x (%s) at > >>> 0x%016llx\n", > >>> - fault_status & 0xFF, > >>> panfrost_exception_name(pfdev, fault_status), > >>> + fault_status & 0xFF, > >>> panfrost_exception_name(pfdev, fault_status & 0xFF), > >> > >> However this change is correct - panfrost_exception_name() should > >> be taking only the lower 8 bits. Even better though would be to to > >> report the full raw fault information as well as the high bits can > >> contain useful information: > >> > >> dev_warn(pfdev->dev, "GPU Fault 0x%08x (%s) at > >> 0x%016llx\n", fault_status, > >> panfrost_exception_name(pfdev, fault_status & > >> 0xFF), address); > >> > >> Thanks, > >> > >> Steve > >> > >>> address); > >>> > >>> if (state & GPU_IRQ_MULTIPLE_FAULT) > >>> > > > >
