Re: [PATCH v2] drm/gem-shmem: When drm_gem_object_init failed, should release object
Hi Thomas,
Can I discard the first two patchs, and pull the new code, then
modify and git send-email this patch?
?? Thu, 17 Nov 2022 14:42:36 +0100
Thomas Zimmermann :
> Hi
>
> Am 11.11.22 um 04:38 schrieb ChunyouTang:
> > when goto err_free, the object had init, so it should be release
> > when fail.
> >
> > Signed-off-by: ChunyouTang
> > ---
> > drivers/gpu/drm/drm_gem.c | 19 ---
> > drivers/gpu/drm/drm_gem_shmem_helper.c | 4 +++-
> > include/drm/drm_gem.h | 1 +
> > 3 files changed, 20 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> > index 8b68a3c1e6ab..cba32c46bb05 100644
> > --- a/drivers/gpu/drm/drm_gem.c
> > +++ b/drivers/gpu/drm/drm_gem.c
> > @@ -169,6 +169,21 @@ void drm_gem_private_object_init(struct
> > drm_device *dev, }
> > EXPORT_SYMBOL(drm_gem_private_object_init);
> >
> > +/**
> > + * drm_gem_private_object_fini - Finalize a failed drm_gem_object
> > + * @obj: drm_gem_object
> > + *
> > + * Uninitialize an already allocated GEM object when it
> > initialized failed
> > + */
> > +void drm_gem_private_object_fini(struct drm_gem_object *obj)
> > +{
> > + WARN_ON(obj->dma_buf);
>
> Rather lease this in its original place.
>
> > +
> > + dma_resv_fini(>_resv);
> > + drm_gem_lru_remove(obj);
>
> AFAICT drm_gem_lru_remove() doesn't belong into this function.
>
> > +}
> > +EXPORT_SYMBOL(drm_gem_private_object_fini);
> > +
> > /**
> >* drm_gem_object_handle_free - release resources bound to
> > userspace handles
> >* @obj: GEM object to clean up.
> > @@ -930,14 +945,12 @@ drm_gem_release(struct drm_device *dev,
> > struct drm_file *file_private) void
> > drm_gem_object_release(struct drm_gem_object *obj)
> > {
> > - WARN_ON(obj->dma_buf);
> > + drm_gem_private_object_fini(obj);
> >
> > if (obj->filp)
> > fput(obj->filp);
> >
> > - dma_resv_fini(>_resv);
>
> Please call drm_gem_private_object_fini() here.
>
> > drm_gem_free_mmap_offset(obj);
> > - drm_gem_lru_remove(obj);
>
> Please keep this line here.
>
> Best regards
> Thomas
>
> > }
> > EXPORT_SYMBOL(drm_gem_object_release);
> >
> > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
> > b/drivers/gpu/drm/drm_gem_shmem_helper.c index
> > 35138f8a375c..845e3d5d71eb 100644 ---
> > a/drivers/gpu/drm/drm_gem_shmem_helper.c +++
> > b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -79,8 +79,10 @@
> > __drm_gem_shmem_create(struct drm_device *dev, size_t size, bool
> > private) } else { ret = drm_gem_object_init(dev, obj, size);
> > }
> > - if (ret)
> > + if (ret) {
> > + drm_gem_private_object_fini(obj)
> > goto err_free;
> > + }
> >
> > ret = drm_gem_create_mmap_offset(obj);
> > if (ret)
> > diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> > index bd42f25e449c..9b1feb03069d 100644
> > --- a/include/drm/drm_gem.h
> > +++ b/include/drm/drm_gem.h
> > @@ -405,6 +405,7 @@ int drm_gem_object_init(struct drm_device *dev,
> > struct drm_gem_object *obj, size_t size);
> > void drm_gem_private_object_init(struct drm_device *dev,
> > struct drm_gem_object *obj,
> > size_t size); +void drm_gem_private_object_fini(struct
> > drm_gem_object *obj); void drm_gem_vm_open(struct vm_area_struct
> > *vma); void drm_gem_vm_close(struct vm_area_struct *vma);
> > int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long
> > obj_size,
>
Re: [PATCH] drm/gem-shmem: When drm_gem_object_init failed, should release object
Hi,
drm_gem_object_init() will do these before failed:
void drm_gem_private_object_init(struct drm_device *dev,
struct drm_gem_object *obj, size_t
size) {
BUG_ON((size & (PAGE_SIZE - 1)) != 0);
obj->dev = dev;
obj->filp = NULL;
kref_init(>refcount);
obj->handle_count = 0;
obj->size = size;
dma_resv_init(>_resv);
if (!obj->resv)
obj->resv = >_resv;
drm_vma_node_reset(>vma_node);
INIT_LIST_HEAD(>lru_node);
}
so I think when it failed, should use drm_gem_object_release()
to do dma_resv_fini() and others.
another, if drm_gem_object_init() fails, the drm_gem_handle_create()
can not be called.
?? Tue, 8 Nov 2022 09:28:34 +0100
Thomas Zimmermann :
> Hi
>
> Am 08.11.22 um 03:03 schrieb ChunyouTang:
> > when goto err_free, the object had init, so it should be release
> > when fail.
>
> If the call to drm_gem_object_init() fails, the object is still
> uninitialized. Admittedly, the call to gem_create_object could need
> additional cleanup, but it appears as if no one has had a need for
> this so far.
>
> Is there anything that might leak?
>
> >
> > Signed-off-by: ChunyouTang
> > ---
> > drivers/gpu/drm/drm_gem_shmem_helper.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
> > b/drivers/gpu/drm/drm_gem_shmem_helper.c index
> > 35138f8a375c..2e5e3207355f 100644 ---
> > a/drivers/gpu/drm/drm_gem_shmem_helper.c +++
> > b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -104,10 +104,10 @@
> > __drm_gem_shmem_create(struct drm_device *dev, size_t size, bool
> > private) return shmem;
> >
> > -err_release:
> > - drm_gem_object_release(obj);
> > err_free:
> > kfree(obj);
> > +err_release:
> > + drm_gem_object_release(obj);
>
> You have now freed the object's memory before releasing it. Not going
> to work.
>
> Best regards
> Thomas
>
> >
> > return ERR_PTR(ret);
> > }
>
Re: [PATCH v2] drm/panfrost:report the full raw fault information instead
Hi Steve, > You didn't answer my previous question: > > > Is this device working with the kbase/DDK proprietary driver? I don't know whether I used kbase/DDK,I only know I used the driver of panfrost in linux 5.11. > What you are describing sounds like a hardware integration issue, so > it would be good to check that the hardware is working with the > proprietary driver to rule that out. And perhaps there is something > in the kbase for this device that is setting a chicken bit to 'fix' > the coherency? I don't have the proprietary driver,I only used driver in linux 5.11. Thinks very much! Chunyou. ?? Thu, 1 Jul 2021 11:15:14 +0100 Steven Price ????: > On 29/06/2021 04:04, Chunyou Tang wrote: > > Hi Steve, > > thinks for your reply. > > I set the pte in arm_lpae_prot_to_pte(), > > *** > > /* > > * Also Mali has its own notions of shareability wherein its > > Inner > > * domain covers the cores within the GPU, and its Outer > > domain is > > * "outside the GPU" (i.e. either the Inner or System > > domain in CPU > > * terms, depending on coherency). > > */ > > if (prot & IOMMU_CACHE && data->iop.fmt != ARM_MALI_LPAE) > > pte |= ARM_LPAE_PTE_SH_IS; > > else > > pte |= ARM_LPAE_PTE_SH_OS; > > *** > > I set pte |= ARM_LPAE_PTE_SH_NS. > > > > If I set pte to ARM_LPAE_PTE_SH_OS or > > ARM_LPAE_PTE_SH_IS,whether I use singel core GPU or multi > > core GPU,it will occur GPU Fault. > > if I set pte to ARM_LPAE_PTE_SH_NS,whether I use singel core > > GPU or multi core GPU,it will not occur GPU Fault. > > Hi, > > So this is a difference between Panfrost and kbase. Panfrost (well > technically the IOMMU framework) enables the inner-shareable bit for > all memory, whereas kbase only enables it for some memory types (the > BASE_MEM_COHERENT_LOCAL flag in the UABI controls it). However this > should only be a performance/power difference (and AFAIK probably an > irrelevant one) and it's definitely required that "inner shareable" > (i.e. within the GPU) works for communication between the different > units of the GPU. > > You didn't answer my previous question: > > > Is this device working with the kbase/DDK proprietary driver? > > What you are describing sounds like a hardware integration issue, so > it would be good to check that the hardware is working with the > proprietary driver to rule that out. And perhaps there is something > in the kbase for this device that is setting a chicken bit to 'fix' > the coherency? > > Steve
Re: [PATCH v2] drm/panfrost:report the full raw fault information instead
Hi Robin,
thinks for you reply.
I had send more detal to Steve in another mail,thinks for your
messages.
?? Mon, 28 Jun 2021 15:17:51 +0100
Robin Murphy :
> On 2021-06-28 11:48, Steven Price wrote:
> > On 25/06/2021 10:49, Chunyou Tang wrote:
> >> Hi Steve,
> >>Thinks for your reply.
> >>When I only set the pte |= ARM_LPAE_PTE_SH_NS;there have
> >> no "GPU Fault",When I set the pte |= ARM_LPAE_PTE_SH_IS(or
> >> ARM_LPAE_PTE_SH_OS);there have "GPU Fault".I don't know how the pte
> >> effect this issue?
> >>Can you give me some suggestions again?
> >>
> >> Thinks.
> >>
> >> Chunyou
> >
> > Hi Chunyou,
> >
> > You haven't given me much context so I'm not entirely sure which
> > PTE you are talking about (GPU or CPU), or indeed where you are
> > changing the PTE values.
> >
> > The PTEs control whether a page is shareable or not, the GPU
> > requires that accesses are consistent (i.e. either all accesses to
> > a page are shareable or all are non-shareable) and will race a
> > fault if it detects this isn't the case. Mali also has a quirk for
> > its version of 'LPAE' where inner shareable actually means only
> > within the GPU and outer shareable means outside the GPU (which I
> > think usually means Inner Shareable on the external bus).
>
> Furthermore, the way the io-pgtable code works for ARM_MALI_LPAE
> format means that *all* GPU mappings are unconditionally
> outer-shareable, so it's not clear how the GPU could observe a
> mismatch in the first place (other than major integration issues
> causing data corruption).
>
> Robin.
>
> >
> > Steve
> >
> >> ?? Thu, 24 Jun 2021 14:22:04 +0100
> >> Steven Price :
> >>
> >>> On 22/06/2021 02:40, Chunyou Tang wrote:
> >>>> Hi Steve,
> >>>> I will send a new patch with suitable subject/commit
> >>>> message. But I send a V3 or a new patch?
> >>>
> >>> Send a V3 - it is a new version of this patch.
> >>>
> >>>> I met a bug about the GPU,I have no idea about how to fix
> >>>> it, If you can give me some suggestion,it is perfect.
> >>>>
> >>>> You can see such kernel log:
> >>>>
> >>>> Jun 20 10:20:13 icube kernel: [ 774.566760] mvp_gpu
> >>>> :05:00.0: GPU Fault 0x0088 (SHAREABILITY_FAULT) at
> >>>> 0x0310fd00 Jun 20 10:20:13 icube kernel: [ 774.566764]
> >>>> mvp_gpu :05:00.0: There were multiple GPU faults - some have
> >>>> not been reported Jun 20 10:20:13 icube kernel: [ 774.667542]
> >>>> mvp_gpu :05:00.0: AS_ACTIVE bit stuck Jun 20 10:20:13 icube
> >>>> kernel: [ 774.767900] mvp_gpu :05:00.0: AS_ACTIVE bit stuck
> >>>> Jun 20 10:20:13 icube kernel: [ 774.868546] mvp_gpu
> >>>> :05:00.0: AS_ACTIVE bit stuck Jun 20 10:20:13 icube kernel:
> >>>> [ 774.968910] mvp_gpu :05:00.0: AS_ACTIVE bit stuck Jun 20
> >>>> 10:20:13 icube kernel: [ 775.069251] mvp_gpu :05:00.0:
> >>>> AS_ACTIVE bit stuck Jun 20 10:20:22 icube kernel: [ 783.693971]
> >>>> mvp_gpu :05:00.0: gpu sched timeout, js=1, config=0x7300,
> >>>> status=0x8, head=0x362c900, tail=0x362c100,
> >>>> sched_job=3252fb84
> >>>>
> >>>> In
> >>>> https://lore.kernel.org/dri-devel/20200510165538.19720-1-peron.c...@gmail.com/
> >>>> there had a same bug like mine,and I found you at the mail list,I
> >>>> don't know how it fixed?
> >>>
> >>> The GPU_SHAREABILITY_FAULT error means that a cache line has been
> >>> accessed both as shareable and non-shareable and therefore
> >>> coherency cannot be guaranteed. Although the "multiple GPU
> >>> faults" means that this may not be the underlying cause.
> >>>
> >>> The fact that your dmesg log has PCI style identifiers
> >>> (":05:00.0") suggests this is an unusual platform - I've not
> >>> previously been aware of a Mali device behind PCI. Is this device
> >>> working with the kbase/DDK proprietary driver? It would be worth
> >>> looking at the kbase kernel code for the platform to see if there
> >>> is anything special done for the platform.
> >>>
> >>> From the dmesg logs all I can really tell is that the GPU seem
Re: [PATCH v2] drm/panfrost:report the full raw fault information instead
Hi Steve,
thinks for your reply.
I set the pte in arm_lpae_prot_to_pte(),
***
/*
* Also Mali has its own notions of shareability wherein its
Inner
* domain covers the cores within the GPU, and its Outer domain
is
* "outside the GPU" (i.e. either the Inner or System domain in
CPU
* terms, depending on coherency).
*/
if (prot & IOMMU_CACHE && data->iop.fmt != ARM_MALI_LPAE)
pte |= ARM_LPAE_PTE_SH_IS;
else
pte |= ARM_LPAE_PTE_SH_OS;
***
I set pte |= ARM_LPAE_PTE_SH_NS.
If I set pte to ARM_LPAE_PTE_SH_OS or
ARM_LPAE_PTE_SH_IS,whether I use singel core GPU or multi core
GPU,it will occur GPU Fault.
if I set pte to ARM_LPAE_PTE_SH_NS,whether I use singel core
GPU or multi core GPU,it will not occur GPU Fault.
Thinks
Chunyou
?? Mon, 28 Jun 2021 11:48:59 +0100
Steven Price :
> On 25/06/2021 10:49, Chunyou Tang wrote:
> > Hi Steve,
> > Thinks for your reply.
> > When I only set the pte |= ARM_LPAE_PTE_SH_NS;there have no
> > "GPU Fault",When I set the pte |= ARM_LPAE_PTE_SH_IS(or
> > ARM_LPAE_PTE_SH_OS);there have "GPU Fault".I don't know how the pte
> > effect this issue?
> > Can you give me some suggestions again?
> >
> > Thinks.
> >
> > Chunyou
>
> Hi Chunyou,
>
> You haven't given me much context so I'm not entirely sure which PTE
> you are talking about (GPU or CPU), or indeed where you are changing
> the PTE values.
>
> The PTEs control whether a page is shareable or not, the GPU requires
> that accesses are consistent (i.e. either all accesses to a page are
> shareable or all are non-shareable) and will race a fault if it
> detects this isn't the case. Mali also has a quirk for its version of
> 'LPAE' where inner shareable actually means only within the GPU and
> outer shareable means outside the GPU (which I think usually means
> Inner Shareable on the external bus).
>
> Steve
>
> > ?? Thu, 24 Jun 2021 14:22:04 +0100
> > Steven Price :
> >
> >> On 22/06/2021 02:40, Chunyou Tang wrote:
> >>> Hi Steve,
> >>> I will send a new patch with suitable subject/commit
> >>> message. But I send a V3 or a new patch?
> >>
> >> Send a V3 - it is a new version of this patch.
> >>
> >>> I met a bug about the GPU,I have no idea about how to fix
> >>> it, If you can give me some suggestion,it is perfect.
> >>>
> >>> You can see such kernel log:
> >>>
> >>> Jun 20 10:20:13 icube kernel: [ 774.566760] mvp_gpu :05:00.0:
> >>> GPU Fault 0x0088 (SHAREABILITY_FAULT) at 0x0310fd00
> >>> Jun 20 10:20:13 icube kernel: [ 774.566764] mvp_gpu :05:00.0:
> >>> There were multiple GPU faults - some have not been reported Jun
> >>> 20 10:20:13 icube kernel: [ 774.667542] mvp_gpu :05:00.0:
> >>> AS_ACTIVE bit stuck Jun 20 10:20:13 icube kernel: [ 774.767900]
> >>> mvp_gpu :05:00.0: AS_ACTIVE bit stuck Jun 20 10:20:13 icube
> >>> kernel: [ 774.868546] mvp_gpu :05:00.0: AS_ACTIVE bit stuck
> >>> Jun 20 10:20:13 icube kernel: [ 774.968910] mvp_gpu :05:00.0:
> >>> AS_ACTIVE bit stuck Jun 20 10:20:13 icube kernel: [ 775.069251]
> >>> mvp_gpu :05:00.0: AS_ACTIVE bit stuck Jun 20 10:20:22 icube
> >>> kernel: [ 783.693971] mvp_gpu :05:00.0: gpu sched timeout,
> >>> js=1, config=0x7300, status=0x8, head=0x362c900, tail=0x362c100,
> >>> sched_job=3252fb84
> >>>
> >>> In
> >>> https://lore.kernel.org/dri-devel/20200510165538.19720-1-peron.c...@gmail.com/
> >>> there had a same bug like mine,and I found you at the mail list,I
> >>> don't know how it fixed?
> >>
> >> The GPU_SHAREABILITY_FAULT error means that a cache line has been
> >> accessed both as shareable and non-shareable and therefore
> >> coherency cannot be guaranteed. Although the "multiple GPU faults"
> >> means that this may not be the underlying cause.
> >>
> >> The fact that your dmesg log has PCI style identifiers
> >> (":05:00.0") suggests this is an unusual platform - I've not
> >> previously been aware of a Mali device behind PCI. Is this device
> >> working with the kbase/DDK proprietary driver? It would be worth
> >> looking at the kba
Re: [PATCH v2] drm/panfrost:report the full raw fault information instead
Hi Steve,
Thinks for your reply.
When I only set the pte |= ARM_LPAE_PTE_SH_NS;there have no "GPU
Fault",When I set the pte |= ARM_LPAE_PTE_SH_IS(or
ARM_LPAE_PTE_SH_OS);there have "GPU Fault".I don't know how the pte
effect this issue?
Can you give me some suggestions again?
Thinks.
Chunyou
?? Thu, 24 Jun 2021 14:22:04 +0100
Steven Price :
> On 22/06/2021 02:40, Chunyou Tang wrote:
> > Hi Steve,
> > I will send a new patch with suitable subject/commit
> > message. But I send a V3 or a new patch?
>
> Send a V3 - it is a new version of this patch.
>
> > I met a bug about the GPU,I have no idea about how to fix
> > it, If you can give me some suggestion,it is perfect.
> >
> > You can see such kernel log:
> >
> > Jun 20 10:20:13 icube kernel: [ 774.566760] mvp_gpu :05:00.0:
> > GPU Fault 0x0088 (SHAREABILITY_FAULT) at 0x0310fd00 Jun
> > 20 10:20:13 icube kernel: [ 774.566764] mvp_gpu :05:00.0:
> > There were multiple GPU faults - some have not been reported Jun 20
> > 10:20:13 icube kernel: [ 774.667542] mvp_gpu :05:00.0:
> > AS_ACTIVE bit stuck Jun 20 10:20:13 icube kernel: [ 774.767900]
> > mvp_gpu :05:00.0: AS_ACTIVE bit stuck Jun 20 10:20:13 icube
> > kernel: [ 774.868546] mvp_gpu :05:00.0: AS_ACTIVE bit stuck
> > Jun 20 10:20:13 icube kernel: [ 774.968910] mvp_gpu :05:00.0:
> > AS_ACTIVE bit stuck Jun 20 10:20:13 icube kernel: [ 775.069251]
> > mvp_gpu :05:00.0: AS_ACTIVE bit stuck Jun 20 10:20:22 icube
> > kernel: [ 783.693971] mvp_gpu :05:00.0: gpu sched timeout,
> > js=1, config=0x7300, status=0x8, head=0x362c900, tail=0x362c100,
> > sched_job=3252fb84
> >
> > In
> > https://lore.kernel.org/dri-devel/20200510165538.19720-1-peron.c...@gmail.com/
> > there had a same bug like mine,and I found you at the mail list,I
> > don't know how it fixed?
>
> The GPU_SHAREABILITY_FAULT error means that a cache line has been
> accessed both as shareable and non-shareable and therefore coherency
> cannot be guaranteed. Although the "multiple GPU faults" means that
> this may not be the underlying cause.
>
> The fact that your dmesg log has PCI style identifiers
> (":05:00.0") suggests this is an unusual platform - I've not
> previously been aware of a Mali device behind PCI. Is this device
> working with the kbase/DDK proprietary driver? It would be worth
> looking at the kbase kernel code for the platform to see if there is
> anything special done for the platform.
>
> From the dmesg logs all I can really tell is that the GPU seems
> unhappy about the memory system.
>
> Steve
>
> > I need your help!
> >
> > thinks very much!
> >
> > Chunyou
> >
> > ?? Mon, 21 Jun 2021 11:45:20 +0100
> > Steven Price :
> >
> >> On 19/06/2021 04:18, Chunyou Tang wrote:
> >>> Hi Steve,
> >>> 1,Now I know how to write the subject
> >>> 2,the low 8 bits is the exception type in spec.
> >>>
> >>> and you can see prnfrost_exception_name()
> >>>
> >>> switch (exception_code) {
> >>> /* Non-Fault Status code */
> >>> case 0x00: return "NOT_STARTED/IDLE/OK";
> >>> case 0x01: return "DONE";
> >>> case 0x02: return "INTERRUPTED";
> >>> case 0x03: return "STOPPED";
> >>> case 0x04: return "TERMINATED";
> >>> case 0x08: return "ACTIVE";
> >>>
> >>>
> >>> case 0xD8: return "ACCESS_FLAG";
> >>> case 0xD9 ... 0xDF: return "ACCESS_FLAG";
> >>> case 0xE0 ... 0xE7: return "ADDRESS_SIZE_FAULT";
> >>> case 0xE8 ... 0xEF: return "MEMORY_ATTRIBUTES_FAULT";
> >>> }
> >>> return "UNKNOWN";
> >>> }
> >>>
> >>> the exception_code in case is only 8 bits,so if fault_status
> >>> in panfrost_gpu_irq_handler() don't & 0xFF,it can't get correct
> >>> exception reason,it will be always UNKNOWN.
> >>
> >> Yes, I'm happy with the change - I just need a patch that I can
> >> apply. At the moment this patch only changes the first '0x%08x'
> >> output rather than the call to panfrost_exception_name() as well.
> >> So we just need a patch which does:
> >>
> >> - fault_status & 0xFF, panfrost_exception_name(pfdev,
> >> fault_status),
> >> + fault_stat
Re: [PATCH v2] drm/panfrost:report the full raw fault information instead
Hi Steve,
I will send a new patch with suitable subject/commit message.
But I send a V3 or a new patch?
I met a bug about the GPU,I have no idea about how to fix it,
If you can give me some suggestion,it is perfect.
You can see such kernel log:
Jun 20 10:20:13 icube kernel: [ 774.566760] mvp_gpu :05:00.0: GPU
Fault 0x0088 (SHAREABILITY_FAULT) at 0x0310fd00 Jun 20
10:20:13 icube kernel: [ 774.566764] mvp_gpu :05:00.0: There were
multiple GPU faults - some have not been reported Jun 20 10:20:13 icube
kernel: [ 774.667542] mvp_gpu :05:00.0: AS_ACTIVE bit stuck Jun 20
10:20:13 icube kernel: [ 774.767900] mvp_gpu :05:00.0: AS_ACTIVE
bit stuck Jun 20 10:20:13 icube kernel: [ 774.868546] mvp_gpu
:05:00.0: AS_ACTIVE bit stuck Jun 20 10:20:13 icube kernel:
[ 774.968910] mvp_gpu :05:00.0: AS_ACTIVE bit stuck Jun 20
10:20:13 icube kernel: [ 775.069251] mvp_gpu :05:00.0: AS_ACTIVE
bit stuck Jun 20 10:20:22 icube kernel: [ 783.693971] mvp_gpu
:05:00.0: gpu sched timeout, js=1, config=0x7300, status=0x8,
head=0x362c900, tail=0x362c100, sched_job=3252fb84
In
https://lore.kernel.org/dri-devel/20200510165538.19720-1-peron.c...@gmail.com/
there had a same bug like mine,and I found you at the mail list,I don't
know how it fixed?
I need your help!
thinks very much!
Chunyou
?? Mon, 21 Jun 2021 11:45:20 +0100
Steven Price :
> On 19/06/2021 04:18, Chunyou Tang wrote:
> > Hi Steve,
> > 1,Now I know how to write the subject
> > 2,the low 8 bits is the exception type in spec.
> >
> > and you can see prnfrost_exception_name()
> >
> > switch (exception_code) {
> > /* Non-Fault Status code */
> > case 0x00: return "NOT_STARTED/IDLE/OK";
> > case 0x01: return "DONE";
> > case 0x02: return "INTERRUPTED";
> > case 0x03: return "STOPPED";
> > case 0x04: return "TERMINATED";
> > case 0x08: return "ACTIVE";
> >
> >
> > case 0xD8: return "ACCESS_FLAG";
> > case 0xD9 ... 0xDF: return "ACCESS_FLAG";
> > case 0xE0 ... 0xE7: return "ADDRESS_SIZE_FAULT";
> > case 0xE8 ... 0xEF: return "MEMORY_ATTRIBUTES_FAULT";
> > }
> > return "UNKNOWN";
> > }
> >
> > the exception_code in case is only 8 bits,so if fault_status
> > in panfrost_gpu_irq_handler() don't & 0xFF,it can't get correct
> > exception reason,it will be always UNKNOWN.
>
> Yes, I'm happy with the change - I just need a patch that I can apply.
> At the moment this patch only changes the first '0x%08x' output rather
> than the call to panfrost_exception_name() as well. So we just need a
> patch which does:
>
> - fault_status & 0xFF, panfrost_exception_name(pfdev, fault_status),
> + fault_status, panfrost_exception_name(pfdev, fault_status & 0xFF),
>
> along with a suitable subject/commit message describing the change. If
> you can send me that I can apply it.
>
> Thanks,
>
> Steve
>
> PS. Sorry for going round in circles here - I'm trying to help you get
> setup so you'll be able to contribute patches easily in future. An
> important part of that is ensuring you can send a properly formatted
> patch to the list.
>
> PPS. I'm still not receiving your emails directly. I don't think it's
> a problem at my end because I'm receiving other emails, but if you can
> somehow fix the problem you're likely to receive a faster response.
>
> > ?? Fri, 18 Jun 2021 13:43:24 +0100
> > Steven Price :
> >
> >> On 17/06/2021 07:20, ChunyouTang wrote:
> >>> From: ChunyouTang
> >>>
> >>> of the low 8 bits.
> >>
> >> Please don't split the subject like this. The first line of the
> >> commit should be a (very short) summary of the patch. Then a blank
> >> line and then a longer description of what the purpose of the
> >> patch is and why it's needed.
> >>
> >> Also you previously had this as part of a series (the first part
> >> adding the "& 0xFF" in the panfrost_exception_name() call). I'm not
> >> sure we need two patches for the single line, but as it stands this
> >> patch doesn't apply.
> >>
> >> Also I'm still not receiving any emails from you directly (only via
> >> the list), so it's possible I might have missed something you sent.
> >>
> >> Steve
> >>
> >>>
> >>> Signed-off-by: ChunyouTang
> >>> ---
> >>> drivers/gpu/drm/panfrost/panfrost_gpu.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 del
Re: [PATCH] drm/panfrost:modify 'break' to 'continue' to traverse the circulation
Hi Steve,
I make a mistake about the code branch,I will test it later,
thinks for your reply.
Chunyou
?? Mon, 21 Jun 2021 11:45:18 +0100
Steven Price :
> On 19/06/2021 04:09, Chunyou Tang wrote:
> > Hi Steve,
> > 1,
> > from
> > https://lore.kernel.org/lkml/31644881-134a-2d6e-dddf-e658a3a81...@arm.com/
> > I can see what your sent,I used a wrong email address,Now it
> > correct. 2,
> >>> Unless I'm mistaken the situation where some mappings may be NULL
> >>> is caused by the loop in panfrost_lookup_bos() not completing
> >>> successfully
> >>> (panfrost_gem_mapping_get() returning NULL). In this case if
> >>> mappings[i]
> >>> is NULL then all following mappings must also be NULL. So 'break'
> >>> allows
> >>> us to skip the later ones. Admittedly the performance here isn't
> >>> important so I'm not sure it's worth the optimisation, but AIUI
> >>> this code isn't actually wrong.
> >
> > from panfrost_lookup_bos(),you can see:
> > for (i = 0; i < job->bo_count; i++) {
> > struct panfrost_gem_mapping *mapping;
> >
> > bo = to_panfrost_bo(job->bos[i]);
> > ICUBE_DEBUG_PRINTK("panfrost bo gem handle=0x%x
> > is_dumb=%d\n", bo->gem_handle, bo->is_dumb);
> > if (!bo->is_dumb) {
> >mapping = panfrost_gem_mapping_get(bo, priv);
> >if (!mapping) {
> > ret = -EINVAL;
> > break;
> >}
> >
> > atomic_inc(>gpu_usecount);
> > job->mappings[i] = mapping;
> > } else {
> > atomic_inc(>gpu_usecount);
> > job->mappings[i] = NULL;
> > }
> > }
>
> This code isn't upstream - in drm-misc/drm-misc-next (and all mainline
> kernels from what I can tell) this doesn't have any "is_dumb" test.
> Which branch are you using?
>
> > if bo->is_dumb is TRUE,the job->mappings[i] will set to NULL,and the
> > while will be continue,so if job->mappings[i] is NULL,the following
> > can not be NULL.
>
> I agree that with the above code the panfrost_job_cleanup() would need
> changing. But we don't (currently) have this code upstream, so this
> change doesn't make sense upstream.
>
> Thanks,
>
> Steve
>
> > 3,
> > I've had this problem in our project,the value of is_dumb like
> > these: 0
> > 0
> > 0
> > 1
> > 0
> > 0
> > 0
> > so,when job->mappings[i] is NULL,we can not break the while in
> > panfrost_job_cleanup().
> >
> > thanks
> > Chunyou
> >
> > ?? Fri, 18 Jun 2021 13:43:25 +0100
> > Steven Price :
> >
> >> On 17/06/2021 09:04, ChunyouTang wrote:
> >>> From: ChunyouTang
> >>>
> >>> The 'break' can cause 'Memory manager not clean during takedown'
> >>>
> >>> It cannot use break to finish the circulation,it should use
> >>>
> >>> continue to traverse the circulation.it should put every mapping
> >>>
> >>> which is not NULL.
> >>
> >> You don't appear to have answered my question about whether you've
> >> actually seen this happen (and ideally what circumstances). In my
> >> previous email[1] I explained why I don't think this is needed. You
> >> need to convince me that I've overlooked something.
> >>
> >> Thanks,
> >>
> >> Steve
> >>
> >> [1]
> >> https://lore.kernel.org/r/31644881-134a-2d6e-dddf-e658a3a8176b%40arm.com
> >>
> >>> Signed-off-by: ChunyouTang
> >>> ---
> >>> drivers/gpu/drm/panfrost/panfrost_job.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c
> >>> b/drivers/gpu/drm/panfrost/panfrost_job.c index
> >>> 6003cfeb1322..52bccc1d2d42 100644 ---
> >>> a/drivers/gpu/drm/panfrost/panfrost_job.c +++
> >>> b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -281,7 +281,7 @@
> >>> static void panfrost_job_cleanup(struct kref *ref) if
> >>> (job->mappings) { for (i = 0; i < job->bo_count; i++) {
> >>> if (!job->mappings[i])
> >>> - break;
> >>> + continue;
> >>>
> >>> atomic_dec(>mappings[i]->obj->gpu_usecount);
> >>> panfrost_gem_mapping_put(job->mappings[i]);
> >>>
> >
> >
Re: [PATCH v2] drm/panfrost:report the full raw fault information instead
Hi Steve,
1,Now I know how to write the subject
2,the low 8 bits is the exception type in spec.
and you can see prnfrost_exception_name()
switch (exception_code) {
/* Non-Fault Status code */
case 0x00: return "NOT_STARTED/IDLE/OK";
case 0x01: return "DONE";
case 0x02: return "INTERRUPTED";
case 0x03: return "STOPPED";
case 0x04: return "TERMINATED";
case 0x08: return "ACTIVE";
case 0xD8: return "ACCESS_FLAG";
case 0xD9 ... 0xDF: return "ACCESS_FLAG";
case 0xE0 ... 0xE7: return "ADDRESS_SIZE_FAULT";
case 0xE8 ... 0xEF: return "MEMORY_ATTRIBUTES_FAULT";
}
return "UNKNOWN";
}
the exception_code in case is only 8 bits,so if fault_status
in panfrost_gpu_irq_handler() don't & 0xFF,it can't get correct
exception reason,it will be always UNKNOWN.
?? Fri, 18 Jun 2021 13:43:24 +0100
Steven Price :
> On 17/06/2021 07:20, ChunyouTang wrote:
> > From: ChunyouTang
> >
> > of the low 8 bits.
>
> Please don't split the subject like this. The first line of the commit
> should be a (very short) summary of the patch. Then a blank line and
> then a longer description of what the purpose of the patch is and why
> it's needed.
>
> Also you previously had this as part of a series (the first part
> adding the "& 0xFF" in the panfrost_exception_name() call). I'm not
> sure we need two patches for the single line, but as it stands this
> patch doesn't apply.
>
> Also I'm still not receiving any emails from you directly (only via
> the list), so it's possible I might have missed something you sent.
>
> Steve
>
> >
> > Signed-off-by: ChunyouTang
> > ---
> > 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
> > 1fffb6a0b24f..d2d287bbf4e7 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 & 0xFF),
> > +fault_status,
> > panfrost_exception_name(pfdev, fault_status & 0xFF), address);
> >
> > if (state & GPU_IRQ_MULTIPLE_FAULT)
> >
Re: [PATCH] drm/panfrost:modify 'break' to 'continue' to traverse the circulation
Hi Steve,
1,
from
https://lore.kernel.org/lkml/31644881-134a-2d6e-dddf-e658a3a81...@arm.com/
I can see what your sent,I used a wrong email address,Now it correct.
2,
> >Unless I'm mistaken the situation where some mappings may be NULL is
> >caused by the loop in panfrost_lookup_bos() not completing
> >successfully
> >(panfrost_gem_mapping_get() returning NULL). In this case if
> >mappings[i]
> >is NULL then all following mappings must also be NULL. So 'break'
> >allows
> >us to skip the later ones. Admittedly the performance here isn't
> >important so I'm not sure it's worth the optimisation, but AIUI this
> >code isn't actually wrong.
from panfrost_lookup_bos(),you can see:
for (i = 0; i < job->bo_count; i++) {
struct panfrost_gem_mapping *mapping;
bo = to_panfrost_bo(job->bos[i]);
ICUBE_DEBUG_PRINTK("panfrost bo gem handle=0x%x
is_dumb=%d\n", bo->gem_handle, bo->is_dumb);
if (!bo->is_dumb) {
mapping = panfrost_gem_mapping_get(bo, priv);
if (!mapping) {
ret = -EINVAL;
break;
}
atomic_inc(>gpu_usecount);
job->mappings[i] = mapping;
} else {
atomic_inc(>gpu_usecount);
job->mappings[i] = NULL;
}
}
if bo->is_dumb is TRUE,the job->mappings[i] will set to NULL,and the
while will be continue,so if job->mappings[i] is NULL,the following
can not be NULL.
3,
I've had this problem in our project,the value of is_dumb like these:
0
0
0
1
0
0
0
so,when job->mappings[i] is NULL,we can not break the while in
panfrost_job_cleanup().
thanks
Chunyou
?? Fri, 18 Jun 2021 13:43:25 +0100
Steven Price :
> On 17/06/2021 09:04, ChunyouTang wrote:
> > From: ChunyouTang
> >
> > The 'break' can cause 'Memory manager not clean during takedown'
> >
> > It cannot use break to finish the circulation,it should use
> >
> > continue to traverse the circulation.it should put every mapping
> >
> > which is not NULL.
>
> You don't appear to have answered my question about whether you've
> actually seen this happen (and ideally what circumstances). In my
> previous email[1] I explained why I don't think this is needed. You
> need to convince me that I've overlooked something.
>
> Thanks,
>
> Steve
>
> [1]
> https://lore.kernel.org/r/31644881-134a-2d6e-dddf-e658a3a8176b%40arm.com
>
> > Signed-off-by: ChunyouTang
> > ---
> > drivers/gpu/drm/panfrost/panfrost_job.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c
> > b/drivers/gpu/drm/panfrost/panfrost_job.c index
> > 6003cfeb1322..52bccc1d2d42 100644 ---
> > a/drivers/gpu/drm/panfrost/panfrost_job.c +++
> > b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -281,7 +281,7 @@
> > static void panfrost_job_cleanup(struct kref *ref) if
> > (job->mappings) { for (i = 0; i < job->bo_count; i++) {
> > if (!job->mappings[i])
> > - break;
> > + continue;
> >
> > atomic_dec(>mappings[i]->obj->gpu_usecount);
> > panfrost_gem_mapping_put(job->mappings[i]);
> >
Re: [PATCH] modified: gpu/drm/panfrost/panfrost_gpu.c
Hi steve,
After I send the V2,I found I setting a wrong email
configuration,I hope it doesn't affect the patch submission :)
Did you received my another patch about panfrost_job.c?
Author: tangchunyou
Date: Wed Jun 9 14:44:52 2021 +0800
modified: gpu/drm/panfrost/panfrost_job.c
The 'break' can cause 'Memory manager not clean during takedown'
It cannot use break to finish the circulation,it should use
continue to traverse the circulation.
Signed-off-by: tangchunyou
diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c
b/drivers/gpu/drm/panfrost/panfrost_job.c
index 6003cfeb1322..52bccc1d2d42 100644
--- a/drivers/gpu/drm/panfrost/panfrost_job.c
+++ b/drivers/gpu/drm/panfrost/panfrost_job.c
@@ -281,7 +281,7 @@ static void panfrost_job_cleanup(struct kref *ref)
if (job->mappings) {
for (i = 0; i < job->bo_count; i++) {
if (!job->mappings[i])
- break;
+ continue;
atomic_dec(>mappings[i]->obj->gpu_usecount);
panfrost_gem_mapping_put(job->mappings[i]);
Thank you!
?? Fri, 11 Jun 2021 11:10:16 +0100
Steven Price :
> 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
> > .
>
> 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 :
> >
> >> 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:
> >>> From: tangchunyou
> >>>
> >>> 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
> >>> ---
> >>> 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)
> >>>
> >
> >
Re: [PATCH] modified: gpu/drm/panfrost/panfrost_gpu.c
?? Fri, 11 Jun 2021 11:10:16 +0100 Steven Price : > 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 > > . > > 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 : > > > >> 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 > >>> > >>> 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 > >>> --- > >>> 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) > >>> > > > >
Re: [PATCH] modified: gpu/drm/panfrost/panfrost_gpu.c
Hi Steven, > > 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 . > 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? ?? Thu, 10 Jun 2021 11:41:52 +0100 Steven Price : > 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: > > From: tangchunyou > > > > 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 > > --- > > 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) > >
Re: [PATCH] drivers/video/fbdev:modify 0 to NULL
Hi,Gustavo On Wed, 17 Mar 2021 20:54:41 -0500 "Gustavo A. R. Silva" wrote: > On 3/17/21 21:47, Chunyou Tang wrote: > > > I think "if (info == NULL)" is more intuitive,and there have many > > compare likes "if (info == NULL)" in this file. > > In that case, all those instances should be changed to if (!foo), > instead. > > -- > Gustavo OK,I change it. -- ChunyouTang ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drivers/video/fbdev:modify 0 to NULL
HI,Gustavo
On Wed, 17 Mar 2021 20:41:15 -0500
"Gustavo A. R. Silva" wrote:
> On 3/17/21 21:33, ChunyouTang wrote:
> > From: tangchunyou
> >
> > modify 0 to NULL,info is a pointer,it should be
> >
> > compared with NULL,not 0
> >
> > Signed-off-by: tangchunyou
> > ---
> > drivers/video/fbdev/offb.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/video/fbdev/offb.c b/drivers/video/fbdev/offb.c
> > index 4501e84..cd1042f 100644
> > --- a/drivers/video/fbdev/offb.c
> > +++ b/drivers/video/fbdev/offb.c
> > @@ -412,7 +412,7 @@ static void __init offb_init_fb(const char
> > *name,
> > info = framebuffer_alloc(sizeof(u32) * 16, NULL);
> >
> > - if (info == 0) {
> > + if (info == NULL) {
>
> if (!info) is better.
>
> --
> Gustavo
>
> > release_mem_region(res_start, res_size);
> > return;
> > }
> >
I think "if (info == NULL)" is more intuitive,and there have many
compare likes "if (info == NULL)" in this file.
--
ChunyouTang
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
