Re: [PATCH] drm/amdgpu: unpack dma_fence_chain containers during sync

[Christian König]

Mhm, then I don't know what's going wrong here.

Could be that the fence somehow ends up in a BO dependency.

Pierre do you have some time for testing today? Or could you provide me 
some way to test this?


Christian.

Am 24.11.20 um 03:48 schrieb Pierre-Loup A. Griffais:


I just built my kernel with it and tested Horizon Zero Dawn on stock 
Proton 5.13, and it doesn't seem to change things there.


This pattern looks identical as with before the kernel patch, as far 
as I can tell:


https://imgur.com/a/1fZWgNG

The last purple block is a piece of GPU work on the gfx ring. It's 
been queued by software 0.7ms ago, but doesn't get put on the HW ring 
until right after the previous piece of GPU work completes. The orange 
chunk below is the 'gfx' kernel task executing, to queue it.


Thanks,

 - Pierre-Loup

On 2020-11-23 18:09, Marek Olšák wrote:

Pierre-Loup, does this do what you requested?

Thanks,
Marek

On Mon, Nov 23, 2020 at 3:17 PM Christian König 
> wrote:


That the CPU round trip is gone now.

Christian.

Am 23.11.20 um 20:49 schrieb Marek Olšák:

What is the behavior we should expect?

Marek

On Mon, Nov 23, 2020 at 7:31 AM Christian König
mailto:ckoenig.leichtzumer...@gmail.com>> wrote:

Ping, Pierre/Marek does this change works as expected?

Regards,
Christian.

Am 18.11.20 um 14:20 schrieb Christian König:
> This allows for optimizing the CPU round trip away.
>
> Signed-off-by: Christian König mailto:christian.koe...@amd.com>>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c   |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 27

>   drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h |  1 +
>   3 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 79342976fa76..68f9a4adf5d2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -1014,7 +1014,7 @@ static int
amdgpu_syncobj_lookup_and_add_to_sync(struct
amdgpu_cs_parser *p,
>               return r;
>       }
>
> -     r = amdgpu_sync_fence(>job->sync, fence);
> +     r = amdgpu_sync_fence_chain(>job->sync, fence);
>       dma_fence_put(fence);
>
>       return r;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> index 8ea6c49529e7..d0d64af06f54 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> @@ -28,6 +28,8 @@
>    *    Christian König mailto:christian.koe...@amd.com>>
>    */
>
> +#include 
> +
>   #include "amdgpu.h"
>   #include "amdgpu_trace.h"
>   #include "amdgpu_amdkfd.h"
> @@ -169,6 +171,31 @@ int amdgpu_sync_fence(struct
amdgpu_sync *sync, struct dma_fence *f)
>       return 0;
>   }
>
> +/**
> + * amdgpu_sync_fence_chain - unpack dma_fence_chain and sync
> + *
> + * @sync: sync object to add fence to
> + * @f: potential dma_fence_chain to sync to.
> + *
> + * Add the fences inside the chain to the sync object.
> + */
> +int amdgpu_sync_fence_chain(struct amdgpu_sync *sync,
struct dma_fence *f)
> +{
> +     int r;
> +
> +     dma_fence_chain_for_each(f, f) {
> +             if (dma_fence_is_signaled(f))
> +                     continue;
> +
> +             r = amdgpu_sync_fence(sync, f);
> +             if (r) {
> +                     dma_fence_put(f);
> +                     return r;
> +             }
> +     }
> +     return 0;
> +}
> +
>   /**
>    * amdgpu_sync_vm_fence - remember to sync to this VM fence
>    *
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
> index 7c0fe20c470d..b142175b65b6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
> @@ -48,6 +48,7 @@ struct amdgpu_sync {
>
>   void amdgpu_sync_create(struct amdgpu_sync *sync);
>   int amdgpu_sync_fence(struct amdgpu_sync *sync, struct
dma_fence *f);
> +int amdgpu_sync_fence_chain(struct amdgpu_sync *sync,
struct dma_fence *f);
>   int amdgpu_sync_vm_fence(struct amdgpu_sync *sync,
struct dma_fence *fence);
>   int amdgpu_sync_resv(struct amdgpu_device *adev, struct
amdgpu_sync 

Re: [PATCH v3 07/12] drm/sched: Prevent any job recoveries after device is unplugged.

[Christian König]

Am 24.11.20 um 02:12 schrieb Luben Tuikov:

On 2020-11-23 3:06 a.m., Christian König wrote:

Am 23.11.20 um 06:37 schrieb Andrey Grodzovsky:

On 11/22/20 6:57 AM, Christian König wrote:

Am 21.11.20 um 06:21 schrieb Andrey Grodzovsky:

No point to try recovery if device is gone, it's meaningless.

I think that this should go into the device specific recovery
function and not in the scheduler.


The timeout timer is rearmed here, so this prevents any new recovery
work to restart from here
after drm_dev_unplug was executed from amdgpu_pci_remove.It will not
cover other places like
job cleanup or starting new job but those should stop once the
scheduler thread is stopped later.

Yeah, but this is rather unclean. We should probably return an error
code instead if the timer should be rearmed or not.

Christian, this is exactly my work I told you about
last week on Wednesday in our weekly meeting. And
which I wrote to you in an email last year about this
time.


Yeah, that's why I'm suggesting it here as well.


So what do we do now?


Split your patches into smaller parts and submit them chunk by chunk.

E.g. renames first and then functional changes grouped by area they change.

Regards,
Christian.



I can submit those changes without the last part,
which builds on this change.

I'm still testing the last part and was hoping
to submit it all in one sequence of patches,
after my testing.

Regards,
Luben


Christian.


Andrey



Christian.


Signed-off-by: Andrey Grodzovsky 
---
   drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c |  2 +-
   drivers/gpu/drm/etnaviv/etnaviv_sched.c   |  3 ++-
   drivers/gpu/drm/lima/lima_sched.c |  3 ++-
   drivers/gpu/drm/panfrost/panfrost_job.c   |  2 +-
   drivers/gpu/drm/scheduler/sched_main.c    | 15 ++-
   drivers/gpu/drm/v3d/v3d_sched.c   | 15 ++-
   include/drm/gpu_scheduler.h   |  6 +-
   7 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index d56f402..d0b0021 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -487,7 +487,7 @@ int amdgpu_fence_driver_init_ring(struct
amdgpu_ring *ring,
     r = drm_sched_init(>sched, _sched_ops,
  num_hw_submission, amdgpu_job_hang_limit,
-   timeout, ring->name);
+   timeout, ring->name, >ddev);
   if (r) {
   DRM_ERROR("Failed to create scheduler on ring %s.\n",
     ring->name);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
index cd46c88..7678287 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
@@ -185,7 +185,8 @@ int etnaviv_sched_init(struct etnaviv_gpu *gpu)
     ret = drm_sched_init(>sched, _sched_ops,
    etnaviv_hw_jobs_limit, etnaviv_job_hang_limit,
- msecs_to_jiffies(500), dev_name(gpu->dev));
+ msecs_to_jiffies(500), dev_name(gpu->dev),
+ gpu->drm);
   if (ret)
   return ret;
   diff --git a/drivers/gpu/drm/lima/lima_sched.c
b/drivers/gpu/drm/lima/lima_sched.c
index dc6df9e..8a7e5d7ca 100644
--- a/drivers/gpu/drm/lima/lima_sched.c
+++ b/drivers/gpu/drm/lima/lima_sched.c
@@ -505,7 +505,8 @@ int lima_sched_pipe_init(struct lima_sched_pipe
*pipe, const char *name)
     return drm_sched_init(>base, _sched_ops, 1,
     lima_job_hang_limit, msecs_to_jiffies(timeout),
-  name);
+  name,
+  pipe->ldev->ddev);
   }
     void lima_sched_pipe_fini(struct lima_sched_pipe *pipe)
diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c
b/drivers/gpu/drm/panfrost/panfrost_job.c
index 30e7b71..37b03b01 100644
--- a/drivers/gpu/drm/panfrost/panfrost_job.c
+++ b/drivers/gpu/drm/panfrost/panfrost_job.c
@@ -520,7 +520,7 @@ int panfrost_job_init(struct panfrost_device
*pfdev)
   ret = drm_sched_init(>queue[j].sched,
    _sched_ops,
    1, 0, msecs_to_jiffies(500),
- "pan_js");
+ "pan_js", pfdev->ddev);
   if (ret) {
   dev_err(pfdev->dev, "Failed to create scheduler: %d.",
ret);
   goto err_sched;
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index c3f0bd0..95db8c6 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -53,6 +53,7 @@
   #include 
   #include 
   #include 
+#include 
     #define CREATE_TRACE_POINTS
   #include "gpu_scheduler_trace.h"
@@ -283,8 +284,16 @@ static void drm_sched_job_timedout(struct
work_struct *work)
   struct drm_gpu_scheduler *sched;
   struct drm_sched_job *job;
   +    int idx;
+
   sched = container_of(work, struct drm_gpu_scheduler,
work_tdr.work);
   +   

Re: [PATCH v3 05/12] drm/ttm: Expose ttm_tt_unpopulate for driver use

[Christian König]

Am 23.11.20 um 22:08 schrieb Andrey Grodzovsky:


On 11/23/20 3:41 PM, Christian König wrote:

Am 23.11.20 um 21:38 schrieb Andrey Grodzovsky:


On 11/23/20 3:20 PM, Christian König wrote:

Am 23.11.20 um 21:05 schrieb Andrey Grodzovsky:


On 11/25/20 5:42 AM, Christian König wrote:

Am 21.11.20 um 06:21 schrieb Andrey Grodzovsky:

It's needed to drop iommu backed pages on device unplug
before device's IOMMU group is released.


It would be cleaner if we could do the whole handling in TTM. I 
also need to double check what you are doing with this function.


Christian.



Check patch "drm/amdgpu: Register IOMMU topology notifier per 
device." to see
how i use it. I don't see why this should go into TTM mid-layer - 
the stuff I do inside
is vendor specific and also I don't think TTM is explicitly aware 
of IOMMU ?
Do you mean you prefer the IOMMU notifier to be registered from 
within TTM

and then use a hook to call into vendor specific handler ?


No, that is really vendor specific.

What I meant is to have a function like 
ttm_resource_manager_evict_all() which you only need to call and 
all tt objects are unpopulated.



So instead of this BO list i create and later iterate in amdgpu from 
the IOMMU patch you just want to do it within

TTM with a single function ? Makes much more sense.


Yes, exactly.

The list_empty() checks we have in TTM for the LRU are actually not 
the best idea, we should now check the pin_count instead. This way we 
could also have a list of the pinned BOs in TTM.



So from my IOMMU topology handler I will iterate the TTM LRU for the 
unpinned BOs and this new function for the pinned ones  ?
It's probably a good idea to combine both iterations into this new 
function to cover all the BOs allocated on the device.


Yes, that's what I had in my mind as well.






BTW: Have you thought about what happens when we unpopulate a BO 
while we still try to use a kernel mapping for it? That could have 
unforeseen consequences.



Are you asking what happens to kmap or vmap style mapped CPU accesses 
once we drop all the DMA backing pages for a particular BO ? Because 
for user mappings
(mmap) we took care of this with dummy page reroute but indeed nothing 
was done for in kernel CPU mappings.


Yes exactly that.

In other words what happens if we free the ring buffer while the kernel 
still writes to it?


Christian.



Andrey




Christian.



Andrey




Give me a day or two to look into this.

Christian.



Andrey






Signed-off-by: Andrey Grodzovsky 
---
  drivers/gpu/drm/ttm/ttm_tt.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c 
b/drivers/gpu/drm/ttm/ttm_tt.c

index 1ccf1ef..29248a5 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -495,3 +495,4 @@ void ttm_tt_unpopulate(struct ttm_tt *ttm)
  else
  ttm_pool_unpopulate(ttm);
  }
+EXPORT_SYMBOL(ttm_tt_unpopulate);



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=04%7C01%7CAndrey.Grodzovsky%40amd.com%7C9be029f26a4746347a6108d88fed299b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637417596065559955%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=tZ3do%2FeKzBtRlNaFbBjCtRvUHKdvwDZ7SoYhEBu4%2BT8%3Dreserved=0 







___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: unpack dma_fence_chain containers during sync

[Pierre-Loup A. Griffais]

I just built my kernel with it and tested Horizon Zero Dawn on stock 
Proton 5.13, and it doesn't seem to change things there.


This pattern looks identical as with before the kernel patch, as far as 
I can tell:


https://imgur.com/a/1fZWgNG

The last purple block is a piece of GPU work on the gfx ring. It's been 
queued by software 0.7ms ago, but doesn't get put on the HW ring until 
right after the previous piece of GPU work completes. The orange chunk 
below is the 'gfx' kernel task executing, to queue it.


Thanks,

 - Pierre-Loup

On 2020-11-23 18:09, Marek Olšák wrote:

Pierre-Loup, does this do what you requested?

Thanks,
Marek

On Mon, Nov 23, 2020 at 3:17 PM Christian König 
> wrote:


That the CPU round trip is gone now.

Christian.

Am 23.11.20 um 20:49 schrieb Marek Olšák:

What is the behavior we should expect?

Marek

On Mon, Nov 23, 2020 at 7:31 AM Christian König
mailto:ckoenig.leichtzumer...@gmail.com>> wrote:

Ping, Pierre/Marek does this change works as expected?

Regards,
Christian.

Am 18.11.20 um 14:20 schrieb Christian König:
> This allows for optimizing the CPU round trip away.
>
> Signed-off-by: Christian König mailto:christian.koe...@amd.com>>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c   |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 27

>   drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h |  1 +
>   3 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 79342976fa76..68f9a4adf5d2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -1014,7 +1014,7 @@ static int
amdgpu_syncobj_lookup_and_add_to_sync(struct amdgpu_cs_parser *p,
>               return r;
>       }
>
> -     r = amdgpu_sync_fence(>job->sync, fence);
> +     r = amdgpu_sync_fence_chain(>job->sync, fence);
>       dma_fence_put(fence);
>
>       return r;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> index 8ea6c49529e7..d0d64af06f54 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> @@ -28,6 +28,8 @@
>    *    Christian König mailto:christian.koe...@amd.com>>
>    */
>
> +#include 
> +
>   #include "amdgpu.h"
>   #include "amdgpu_trace.h"
>   #include "amdgpu_amdkfd.h"
> @@ -169,6 +171,31 @@ int amdgpu_sync_fence(struct
amdgpu_sync *sync, struct dma_fence *f)
>       return 0;
>   }
>
> +/**
> + * amdgpu_sync_fence_chain - unpack dma_fence_chain and sync
> + *
> + * @sync: sync object to add fence to
> + * @f: potential dma_fence_chain to sync to.
> + *
> + * Add the fences inside the chain to the sync object.
> + */
> +int amdgpu_sync_fence_chain(struct amdgpu_sync *sync,
struct dma_fence *f)
> +{
> +     int r;
> +
> +     dma_fence_chain_for_each(f, f) {
> +             if (dma_fence_is_signaled(f))
> +                     continue;
> +
> +             r = amdgpu_sync_fence(sync, f);
> +             if (r) {
> +                     dma_fence_put(f);
> +                     return r;
> +             }
> +     }
> +     return 0;
> +}
> +
>   /**
>    * amdgpu_sync_vm_fence - remember to sync to this VM fence
>    *
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
> index 7c0fe20c470d..b142175b65b6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
> @@ -48,6 +48,7 @@ struct amdgpu_sync {
>
>   void amdgpu_sync_create(struct amdgpu_sync *sync);
>   int amdgpu_sync_fence(struct amdgpu_sync *sync, struct
dma_fence *f);
> +int amdgpu_sync_fence_chain(struct amdgpu_sync *sync,
struct dma_fence *f);
>   int amdgpu_sync_vm_fence(struct amdgpu_sync *sync, struct
dma_fence *fence);
>   int amdgpu_sync_resv(struct amdgpu_device *adev, struct
amdgpu_sync *sync,
>                    struct dma_resv *resv, enum
amdgpu_sync_mode mode,

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org

[PATCH] drm/amdgpu: skip power profile switch in sriov

[Jingwen Chen]

power profile switch in vcn need to send SetWorkLoad msg to
smu, which is not supported in sriov.

Signed-off-by: Jingwen Chen 
---
 drivers/gpu/drm/amd/pm/amdgpu_dpm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c 
b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
index 17a45baff638..8fb12afe3c96 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
@@ -1168,6 +1168,9 @@ int amdgpu_dpm_switch_power_profile(struct amdgpu_device 
*adev,
 {
int ret = 0;
 
+   if (amdgpu_sriov_vf(adev))
+   return 0;
+
if (is_support_sw_smu(adev))
ret = smu_switch_power_profile(>smu, type, en);
else if (adev->powerplay.pp_funcs &&
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 1/1] drm/amdgpu: only skip smc sdma sos ta and asd fw in SRIOV for navi12

[Yang, Stanley]

[AMD Public Use]

Hi Hawking,

Thanks for your reminding, it is unnecessary to check navi12 asic type in sdma 
v4, I will update patch and resend it.

Regards,
Stanley

> -Original Message-
> From: Zhang, Hawking 
> Sent: Monday, November 23, 2020 9:31 PM
> To: Yang, Stanley ; amd-gfx@lists.freedesktop.org;
> Chen, JingWen 
> Cc: Yang, Stanley 
> Subject: RE: [PATCH 1/1] drm/amdgpu: only skip smc sdma sos ta and asd fw
> in SRIOV for navi12
> 
> [AMD Public Use]
> 
> @@ -593,7 +593,7 @@ static int sdma_v4_0_init_microcode(struct
> amdgpu_device *adev)
>   struct amdgpu_firmware_info *info = NULL;
>   const struct common_firmware_header *header = NULL;
> 
> - if (amdgpu_sriov_vf(adev))
> + if (amdgpu_sriov_vf(adev) && (adev->asic_type == CHIP_NAVI12))
> 
> Navi12 doesn't integrate sdma v4. Why we need to check Navi12 in sdma v4
> function.
> 
> Regards,
> Hawking
> 
> -Original Message-
> From: amd-gfx  On Behalf Of
> Stanley.Yang
> Sent: Monday, November 23, 2020 21:19
> To: amd-gfx@lists.freedesktop.org; Chen, JingWen
> 
> Cc: Yang, Stanley 
> Subject: [PATCH 1/1] drm/amdgpu: only skip smc sdma sos ta and asd fw in
> SRIOV for navi12
> 
> The KFDTopologyTest.BasicTest will failed if skip smc, sdma, sos, ta and asd
> fw in SRIOV for vega10, so adjust above fw and skip load them in SRIOV only
> for navi12.
> 
> Signed-off-by: Stanley.Yang 
> Change-Id: Id354be93723d7b5d769d73dc67c596af300305af
> ---
>  drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c  | 2 +-
>  drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c  | 2 +-
>  drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c  | 2 +-
>  drivers/gpu/drm/amd/pm/powerplay/smumgr/vega10_smumgr.c | 3 ++-
>  drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c   | 2 +-
>  5 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> index 16b551f330a4..7e2f063120d8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -593,7 +593,7 @@ static int sdma_v4_0_init_microcode(struct
> amdgpu_device *adev)
>   struct amdgpu_firmware_info *info = NULL;
>   const struct common_firmware_header *header = NULL;
> 
> - if (amdgpu_sriov_vf(adev))
> + if (amdgpu_sriov_vf(adev) && (adev->asic_type == CHIP_NAVI12))
>   return 0;
> 
>   DRM_DEBUG("\n");
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> index 9c72b95b7463..fad1cc394219 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> @@ -203,7 +203,7 @@ static int sdma_v5_0_init_microcode(struct
> amdgpu_device *adev)
>   const struct common_firmware_header *header = NULL;
>   const struct sdma_firmware_header_v1_0 *hdr;
> 
> - if (amdgpu_sriov_vf(adev))
> + if (amdgpu_sriov_vf(adev) && (adev->asic_type == CHIP_NAVI12))
>   return 0;
> 
>   DRM_DEBUG("\n");
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> index cb5a6f1437f8..674bc88c3ec1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> @@ -153,7 +153,7 @@ static int sdma_v5_2_init_microcode(struct
> amdgpu_device *adev)
>   struct amdgpu_firmware_info *info = NULL;
>   const struct common_firmware_header *header = NULL;
> 
> - if (amdgpu_sriov_vf(adev))
> + if (amdgpu_sriov_vf(adev) && (adev->asic_type == CHIP_NAVI12))
>   return 0;
> 
>   DRM_DEBUG("\n");
> diff --git a/drivers/gpu/drm/amd/pm/powerplay/smumgr/vega10_smumgr.c
> b/drivers/gpu/drm/amd/pm/powerplay/smumgr/vega10_smumgr.c
> index daf122f24f23..192149e94f6c 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/smumgr/vega10_smumgr.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/smumgr/vega10_smumgr.c
> @@ -208,8 +208,9 @@ static int vega10_smu_init(struct pp_hwmgr *hwmgr)
>   unsigned long tools_size;
>   int ret;
>   struct cgs_firmware_info info = {0};
> + struct amdgpu_device *adev = hwmgr->adev;
> 
> - if (!amdgpu_sriov_vf((struct amdgpu_device *)hwmgr->adev)) {
> + if (!amdgpu_sriov_vf(adev) || (adev->asic_type != CHIP_NAVI12)) {
>   ret = cgs_get_firmware_info(hwmgr->device,
>   CGS_UCODE_ID_SMU,
>   );
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index 1904df5a3e20..80c0bfaed097 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -847,7 +847,7 @@ static int smu_sw_init(void *handle)
>   smu->smu_dpm.dpm_level = AMD_DPM_FORCED_LEVEL_AUTO;
>   smu->smu_dpm.requested_dpm_level =
> AMD_DPM_FORCED_LEVEL_AUTO;
> 
> - if (!amdgpu_sriov_vf(adev)) {
> + if (!amdgpu_sriov_vf(adev) || (adev->asic_type 

Re: [PATCH] drm/amdgpu: unpack dma_fence_chain containers during sync

[Marek Olšák]

Pierre-Loup, does this do what you requested?

Thanks,
Marek

On Mon, Nov 23, 2020 at 3:17 PM Christian König <
ckoenig.leichtzumer...@gmail.com> wrote:

> That the CPU round trip is gone now.
>
> Christian.
>
> Am 23.11.20 um 20:49 schrieb Marek Olšák:
>
> What is the behavior we should expect?
>
> Marek
>
> On Mon, Nov 23, 2020 at 7:31 AM Christian König <
> ckoenig.leichtzumer...@gmail.com> wrote:
>
>> Ping, Pierre/Marek does this change works as expected?
>>
>> Regards,
>> Christian.
>>
>> Am 18.11.20 um 14:20 schrieb Christian König:
>> > This allows for optimizing the CPU round trip away.
>> >
>> > Signed-off-by: Christian König 
>> > ---
>> >   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c   |  2 +-
>> >   drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 27 
>> >   drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h |  1 +
>> >   3 files changed, 29 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> > index 79342976fa76..68f9a4adf5d2 100644
>> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> > @@ -1014,7 +1014,7 @@ static int
>> amdgpu_syncobj_lookup_and_add_to_sync(struct amdgpu_cs_parser *p,
>> >   return r;
>> >   }
>> >
>> > - r = amdgpu_sync_fence(>job->sync, fence);
>> > + r = amdgpu_sync_fence_chain(>job->sync, fence);
>> >   dma_fence_put(fence);
>> >
>> >   return r;
>> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
>> > index 8ea6c49529e7..d0d64af06f54 100644
>> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
>> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
>> > @@ -28,6 +28,8 @@
>> >*Christian König 
>> >*/
>> >
>> > +#include 
>> > +
>> >   #include "amdgpu.h"
>> >   #include "amdgpu_trace.h"
>> >   #include "amdgpu_amdkfd.h"
>> > @@ -169,6 +171,31 @@ int amdgpu_sync_fence(struct amdgpu_sync *sync,
>> struct dma_fence *f)
>> >   return 0;
>> >   }
>> >
>> > +/**
>> > + * amdgpu_sync_fence_chain - unpack dma_fence_chain and sync
>> > + *
>> > + * @sync: sync object to add fence to
>> > + * @f: potential dma_fence_chain to sync to.
>> > + *
>> > + * Add the fences inside the chain to the sync object.
>> > + */
>> > +int amdgpu_sync_fence_chain(struct amdgpu_sync *sync, struct dma_fence
>> *f)
>> > +{
>> > + int r;
>> > +
>> > + dma_fence_chain_for_each(f, f) {
>> > + if (dma_fence_is_signaled(f))
>> > + continue;
>> > +
>> > + r = amdgpu_sync_fence(sync, f);
>> > + if (r) {
>> > + dma_fence_put(f);
>> > + return r;
>> > + }
>> > + }
>> > + return 0;
>> > +}
>> > +
>> >   /**
>> >* amdgpu_sync_vm_fence - remember to sync to this VM fence
>> >*
>> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
>> > index 7c0fe20c470d..b142175b65b6 100644
>> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
>> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
>> > @@ -48,6 +48,7 @@ struct amdgpu_sync {
>> >
>> >   void amdgpu_sync_create(struct amdgpu_sync *sync);
>> >   int amdgpu_sync_fence(struct amdgpu_sync *sync, struct dma_fence *f);
>> > +int amdgpu_sync_fence_chain(struct amdgpu_sync *sync, struct dma_fence
>> *f);
>> >   int amdgpu_sync_vm_fence(struct amdgpu_sync *sync, struct dma_fence
>> *fence);
>> >   int amdgpu_sync_resv(struct amdgpu_device *adev, struct amdgpu_sync
>> *sync,
>> >struct dma_resv *resv, enum amdgpu_sync_mode mode,
>>
>> ___
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
>
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[Jakub Kicinski]

On Mon, 23 Nov 2020 17:32:51 -0800 Nick Desaulniers wrote:
> On Sun, Nov 22, 2020 at 8:17 AM Kees Cook  wrote:
> > On Fri, Nov 20, 2020 at 11:51:42AM -0800, Jakub Kicinski wrote:  
> > > If none of the 140 patches here fix a real bug, and there is no change
> > > to machine code then it sounds to me like a W=2 kind of a warning.  
> >
> > FWIW, this series has found at least one bug so far:
> > https://lore.kernel.org/lkml/CAFCwf11izHF=g1mGry1fE5kvFFFrxzhPSM6qKAO8gxSp=kr...@mail.gmail.com/
> >   
> 
> So looks like the bulk of these are:
> switch (x) {
>   case 0:
> ++x;
>   default:
> break;
> }
> 
> I have a patch that fixes those up for clang:
> https://reviews.llvm.org/D91895


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v3 07/12] drm/sched: Prevent any job recoveries after device is unplugged.

[Luben Tuikov]

On 2020-11-23 3:06 a.m., Christian König wrote:
> Am 23.11.20 um 06:37 schrieb Andrey Grodzovsky:
>>
>> On 11/22/20 6:57 AM, Christian König wrote:
>>> Am 21.11.20 um 06:21 schrieb Andrey Grodzovsky:
 No point to try recovery if device is gone, it's meaningless.
>>>
>>> I think that this should go into the device specific recovery 
>>> function and not in the scheduler.
>>
>>
>> The timeout timer is rearmed here, so this prevents any new recovery 
>> work to restart from here
>> after drm_dev_unplug was executed from amdgpu_pci_remove.It will not 
>> cover other places like
>> job cleanup or starting new job but those should stop once the 
>> scheduler thread is stopped later.
> 
> Yeah, but this is rather unclean. We should probably return an error 
> code instead if the timer should be rearmed or not.

Christian, this is exactly my work I told you about
last week on Wednesday in our weekly meeting. And
which I wrote to you in an email last year about this
time.

So what do we do now?

I can submit those changes without the last part,
which builds on this change.

I'm still testing the last part and was hoping
to submit it all in one sequence of patches,
after my testing.

Regards,
Luben

> 
> Christian.
> 
>>
>> Andrey
>>
>>
>>>
>>> Christian.
>>>

 Signed-off-by: Andrey Grodzovsky 
 ---
   drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c |  2 +-
   drivers/gpu/drm/etnaviv/etnaviv_sched.c   |  3 ++-
   drivers/gpu/drm/lima/lima_sched.c |  3 ++-
   drivers/gpu/drm/panfrost/panfrost_job.c   |  2 +-
   drivers/gpu/drm/scheduler/sched_main.c    | 15 ++-
   drivers/gpu/drm/v3d/v3d_sched.c   | 15 ++-
   include/drm/gpu_scheduler.h   |  6 +-
   7 files changed, 35 insertions(+), 11 deletions(-)

 diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c 
 b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
 index d56f402..d0b0021 100644
 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
 @@ -487,7 +487,7 @@ int amdgpu_fence_driver_init_ring(struct 
 amdgpu_ring *ring,
     r = drm_sched_init(>sched, _sched_ops,
  num_hw_submission, amdgpu_job_hang_limit,
 -   timeout, ring->name);
 +   timeout, ring->name, >ddev);
   if (r) {
   DRM_ERROR("Failed to create scheduler on ring %s.\n",
     ring->name);
 diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c 
 b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
 index cd46c88..7678287 100644
 --- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
 +++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
 @@ -185,7 +185,8 @@ int etnaviv_sched_init(struct etnaviv_gpu *gpu)
     ret = drm_sched_init(>sched, _sched_ops,
    etnaviv_hw_jobs_limit, etnaviv_job_hang_limit,
 - msecs_to_jiffies(500), dev_name(gpu->dev));
 + msecs_to_jiffies(500), dev_name(gpu->dev),
 + gpu->drm);
   if (ret)
   return ret;
   diff --git a/drivers/gpu/drm/lima/lima_sched.c 
 b/drivers/gpu/drm/lima/lima_sched.c
 index dc6df9e..8a7e5d7ca 100644
 --- a/drivers/gpu/drm/lima/lima_sched.c
 +++ b/drivers/gpu/drm/lima/lima_sched.c
 @@ -505,7 +505,8 @@ int lima_sched_pipe_init(struct lima_sched_pipe 
 *pipe, const char *name)
     return drm_sched_init(>base, _sched_ops, 1,
     lima_job_hang_limit, msecs_to_jiffies(timeout),
 -  name);
 +  name,
 +  pipe->ldev->ddev);
   }
     void lima_sched_pipe_fini(struct lima_sched_pipe *pipe)
 diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c 
 b/drivers/gpu/drm/panfrost/panfrost_job.c
 index 30e7b71..37b03b01 100644
 --- a/drivers/gpu/drm/panfrost/panfrost_job.c
 +++ b/drivers/gpu/drm/panfrost/panfrost_job.c
 @@ -520,7 +520,7 @@ int panfrost_job_init(struct panfrost_device 
 *pfdev)
   ret = drm_sched_init(>queue[j].sched,
    _sched_ops,
    1, 0, msecs_to_jiffies(500),
 - "pan_js");
 + "pan_js", pfdev->ddev);
   if (ret) {
   dev_err(pfdev->dev, "Failed to create scheduler: %d.", 
 ret);
   goto err_sched;
 diff --git a/drivers/gpu/drm/scheduler/sched_main.c 
 b/drivers/gpu/drm/scheduler/sched_main.c
 index c3f0bd0..95db8c6 100644
 --- a/drivers/gpu/drm/scheduler/sched_main.c
 +++ b/drivers/gpu/drm/scheduler/sched_main.c
 @@ -53,6 +53,7 @@
   #include 
   #include 
   #include 
 +#include 
     #define CREATE_TRACE_POINTS
   #include "gpu_scheduler_trace.h"
 @@ -283,8 +284,16 @@ static void 

RE: [PATCH] drm/amd/powerplay: fix spelling mistake "smu_state_memroy_block" -> "smu_state_memory_block"

[Quan, Evan]

[AMD Official Use Only - Internal Distribution Only]

Reviewed-by: Evan Quan 

-Original Message-
From: Colin King 
Sent: Monday, November 23, 2020 6:54 PM
To: Deucher, Alexander ; Koenig, Christian 
; David Airlie ; Daniel Vetter 
; Quan, Evan ; Wang, Kevin(Yang) 
; Gui, Jack ; 
amd-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org
Cc: kernel-janit...@vger.kernel.org; linux-ker...@vger.kernel.org
Subject: [PATCH] drm/amd/powerplay: fix spelling mistake 
"smu_state_memroy_block" -> "smu_state_memory_block"

From: Colin Ian King 

The struct name smu_state_memroy_block contains a spelling mistake, rename it 
to smu_state_memory_block

Fixes: 8554e67d6e22 ("drm/amd/powerplay: implement power_dpm_state sys 
interface for SMU11")
Signed-off-by: Colin Ian King 
---
 drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h 
b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
index 7550757cc059..a559ea2204c1 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
@@ -99,7 +99,7 @@ struct smu_state_display_block {
 bool  enable_vari_bright;
 };

-struct smu_state_memroy_block {
+struct smu_state_memory_block {
 bool  dll_off;
 uint8_t m3arb;
 uint8_t unused[3];
@@ -146,7 +146,7 @@ struct smu_power_state {
 struct smu_state_validation_block validation;
 struct smu_state_pcie_block   pcie;
 struct smu_state_display_blockdisplay;
-struct smu_state_memroy_block memory;
+struct smu_state_memory_block memory;
 struct smu_state_software_algorithm_block software;
 struct smu_uvd_clocks uvd_clocks;
 struct smu_hw_power_state hardware;
--
2.28.0

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 028/141] drm/amd/display: Fix fall-through warnings for Clang

[Gustavo A. R. Silva]

On Fri, Nov 20, 2020 at 05:45:07PM -0500, Alex Deucher wrote:
> On Fri, Nov 20, 2020 at 1:28 PM Gustavo A. R. Silva
>  wrote:
> >
> > In preparation to enable -Wimplicit-fallthrough for Clang, fix multiple
> > warnings by explicitly adding multiple break statements instead of just
> > letting the code fall through to the next case.
> >
> > Link: https://github.com/KSPP/linux/issues/115
> > Signed-off-by: Gustavo A. R. Silva 
> 
> Applied.  Thanks!

Thanks, Alex.
--
Gustavo
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 004/141] drm/amdgpu: Fix fall-through warnings for Clang

[Gustavo A. R. Silva]

On Fri, Nov 20, 2020 at 05:42:38PM -0500, Alex Deucher wrote:
> On Fri, Nov 20, 2020 at 1:24 PM Gustavo A. R. Silva
>  wrote:
> >
> > In preparation to enable -Wimplicit-fallthrough for Clang, fix multiple
> > warnings by explicitly adding multiple break statements instead of just
> > letting the code fall through to the next case.
> >
> > Link: https://github.com/KSPP/linux/issues/115
> > Signed-off-by: Gustavo A. R. Silva 
> 
> Applied.  Thanks!

Thanks, Alex.
--
Gustavo
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[James Bottomley]

On Mon, 2020-11-23 at 19:56 +0100, Miguel Ojeda wrote:
> On Mon, Nov 23, 2020 at 4:58 PM James Bottomley
>  wrote:
> > Well, I used git.  It says that as of today in Linus' tree we have
> > 889 patches related to fall throughs and the first series went in
> > in october 2017 ... ignoring a couple of outliers back to February.
> 
> I can see ~10k insertions over ~1k commits and 15 years that mention
> a fallthrough in the entire repo. That is including some commits
> (like the biggest one, 960 insertions) that have nothing to do with C
> fallthrough. A single kernel release has an order of magnitude more
> changes than this...
> 
> But if we do the math, for an author, at even 1 minute per line
> change and assuming nothing can be automated at all, it would take 1
> month of work. For maintainers, a couple of trivial lines is noise
> compared to many other patches.

So you think a one line patch should take one minute to produce ... I
really don't think that's grounded in reality.  I suppose a one line
patch only takes a minute to merge with b4 if no-one reviews or tests
it, but that's not really desirable.

> In fact, this discussion probably took more time than the time it
> would take to review the 200 lines. :-)

I'm framing the discussion in terms of the whole series of changes we
have done for fall through, both what's in the tree currently (889
patches) both in terms of the produce and the consumer.  That's what I
used for my figures for cost.

> > We're also complaining about the inability to recruit maintainers:
> > 
> > https://www.theregister.com/2020/06/30/hard_to_find_linux_maintainers_says_torvalds/
> > 
> > And burn out:
> > 
> > http://antirez.com/news/129
> 
> Accepting trivial and useful 1-line patches

Part of what I'm trying to measure is the "and useful" bit because
that's not a given.

> is not what makes a voluntary maintainer quit...

so the proverb "straw which broke the camel's back" uniquely doesn't
apply to maintainers

>  Thankless work with demanding deadlines is.

That's another potential reason, but it doesn't may other reasons less
valid.

> > The whole crux of your argument seems to be maintainers' time isn't
> > important so we should accept all trivial patches
> 
> I have not said that, at all. In fact, I am a voluntary one and I
> welcome patches like this. It takes very little effort on my side to
> review and it helps the kernel overall.

Well, you know, subsystems are very different in terms of the amount of
patches a maintainer has to process per release cycle of the kernel. 
If a maintainer is close to capacity, additional patches, however
trivial, become a problem.  If a maintainer has spare cycles, trivial
patches may look easy.

> Paid maintainers are the ones that can take care of big
> features/reviews.
> 
> > What I'm actually trying to articulate is a way of measuring value
> > of the patch vs cost ... it has nothing really to do with who foots
> > the actual bill.
> 
> I understand your point, but you were the one putting it in terms of
> a junior FTE.

No, I evaluated the producer side in terms of an FTE.  What we're
mostly arguing about here is the consumer side: the maintainers and
people who have to rework their patch sets. I estimated that at 100h.

>  In my view, 1 month-work (worst case) is very much worth
> removing a class of errors from a critical codebase.
> 
> > One thesis I'm actually starting to formulate is that this
> > continual devaluing of maintainers is why we have so much
> > difficulty keeping and recruiting them.
> 
> That may very well be true, but I don't feel anybody has devalued
> maintainers in this discussion.

You seem to be saying that because you find it easy to merge trivial
patches, everyone should.  I'm reminded of a friend long ago who
thought being a Tees River Pilot was a sinecure because he could
navigate the Tees blindfold.  What he forgot, of course, is that just
because it's easy with a trawler doesn't mean it's easy with an oil
tanker.  In fact it takes longer to qualify as a Tees River Pilot than
it does to get a PhD.

James


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[Jason Gunthorpe]

On Fri, Nov 20, 2020 at 12:21:39PM -0600, Gustavo A. R. Silva wrote:

>   IB/hfi1: Fix fall-through warnings for Clang
>   IB/mlx4: Fix fall-through warnings for Clang
>   IB/qedr: Fix fall-through warnings for Clang
>   RDMA/mlx5: Fix fall-through warnings for Clang

I picked these four to the rdma tree, thanks

Jason
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v3 05/12] drm/ttm: Expose ttm_tt_unpopulate for driver use

[Andrey Grodzovsky]

On 11/23/20 3:41 PM, Christian König wrote:

Am 23.11.20 um 21:38 schrieb Andrey Grodzovsky:


On 11/23/20 3:20 PM, Christian König wrote:

Am 23.11.20 um 21:05 schrieb Andrey Grodzovsky:


On 11/25/20 5:42 AM, Christian König wrote:

Am 21.11.20 um 06:21 schrieb Andrey Grodzovsky:

It's needed to drop iommu backed pages on device unplug
before device's IOMMU group is released.


It would be cleaner if we could do the whole handling in TTM. I also need 
to double check what you are doing with this function.


Christian.



Check patch "drm/amdgpu: Register IOMMU topology notifier per device." to see
how i use it. I don't see why this should go into TTM mid-layer - the stuff 
I do inside

is vendor specific and also I don't think TTM is explicitly aware of IOMMU ?
Do you mean you prefer the IOMMU notifier to be registered from within TTM
and then use a hook to call into vendor specific handler ?


No, that is really vendor specific.

What I meant is to have a function like ttm_resource_manager_evict_all() 
which you only need to call and all tt objects are unpopulated.



So instead of this BO list i create and later iterate in amdgpu from the 
IOMMU patch you just want to do it within

TTM with a single function ? Makes much more sense.


Yes, exactly.

The list_empty() checks we have in TTM for the LRU are actually not the best 
idea, we should now check the pin_count instead. This way we could also have a 
list of the pinned BOs in TTM.



So from my IOMMU topology handler I will iterate the TTM LRU for the unpinned 
BOs and this new function for the pinned ones  ?
It's probably a good idea to combine both iterations into this new function to 
cover all the BOs allocated on the device.





BTW: Have you thought about what happens when we unpopulate a BO while we 
still try to use a kernel mapping for it? That could have unforeseen 
consequences.



Are you asking what happens to kmap or vmap style mapped CPU accesses once we 
drop all the DMA backing pages for a particular BO ? Because for user mappings
(mmap) we took care of this with dummy page reroute but indeed nothing was done 
for in kernel CPU mappings.


Andrey




Christian.



Andrey




Give me a day or two to look into this.

Christian.



Andrey






Signed-off-by: Andrey Grodzovsky 
---
  drivers/gpu/drm/ttm/ttm_tt.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 1ccf1ef..29248a5 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -495,3 +495,4 @@ void ttm_tt_unpopulate(struct ttm_tt *ttm)
  else
  ttm_pool_unpopulate(ttm);
  }
+EXPORT_SYMBOL(ttm_tt_unpopulate);



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=04%7C01%7CAndrey.Grodzovsky%40amd.com%7C9be029f26a4746347a6108d88fed299b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637417596065559955%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=tZ3do%2FeKzBtRlNaFbBjCtRvUHKdvwDZ7SoYhEBu4%2BT8%3Dreserved=0 






___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v3 05/12] drm/ttm: Expose ttm_tt_unpopulate for driver use

[Christian König]

Am 23.11.20 um 21:38 schrieb Andrey Grodzovsky:


On 11/23/20 3:20 PM, Christian König wrote:

Am 23.11.20 um 21:05 schrieb Andrey Grodzovsky:


On 11/25/20 5:42 AM, Christian König wrote:

Am 21.11.20 um 06:21 schrieb Andrey Grodzovsky:

It's needed to drop iommu backed pages on device unplug
before device's IOMMU group is released.


It would be cleaner if we could do the whole handling in TTM. I 
also need to double check what you are doing with this function.


Christian.



Check patch "drm/amdgpu: Register IOMMU topology notifier per 
device." to see
how i use it. I don't see why this should go into TTM mid-layer - 
the stuff I do inside
is vendor specific and also I don't think TTM is explicitly aware of 
IOMMU ?
Do you mean you prefer the IOMMU notifier to be registered from 
within TTM

and then use a hook to call into vendor specific handler ?


No, that is really vendor specific.

What I meant is to have a function like 
ttm_resource_manager_evict_all() which you only need to call and all 
tt objects are unpopulated.



So instead of this BO list i create and later iterate in amdgpu from 
the IOMMU patch you just want to do it within

TTM with a single function ? Makes much more sense.


Yes, exactly.

The list_empty() checks we have in TTM for the LRU are actually not the 
best idea, we should now check the pin_count instead. This way we could 
also have a list of the pinned BOs in TTM.


BTW: Have you thought about what happens when we unpopulate a BO while 
we still try to use a kernel mapping for it? That could have unforeseen 
consequences.


Christian.



Andrey




Give me a day or two to look into this.

Christian.



Andrey






Signed-off-by: Andrey Grodzovsky 
---
  drivers/gpu/drm/ttm/ttm_tt.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c 
b/drivers/gpu/drm/ttm/ttm_tt.c

index 1ccf1ef..29248a5 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -495,3 +495,4 @@ void ttm_tt_unpopulate(struct ttm_tt *ttm)
  else
  ttm_pool_unpopulate(ttm);
  }
+EXPORT_SYMBOL(ttm_tt_unpopulate);



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=04%7C01%7CAndrey.Grodzovsky%40amd.com%7C9be029f26a4746347a6108d88fed299b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637417596065559955%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=tZ3do%2FeKzBtRlNaFbBjCtRvUHKdvwDZ7SoYhEBu4%2BT8%3Dreserved=0 





___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[Mark Brown]

On Fri, 20 Nov 2020 12:21:39 -0600, Gustavo A. R. Silva wrote:
> This series aims to fix almost all remaining fall-through warnings in
> order to enable -Wimplicit-fallthrough for Clang.
> 
> In preparation to enable -Wimplicit-fallthrough for Clang, explicitly
> add multiple break/goto/return/fallthrough statements instead of just
> letting the code fall through to the next case.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 
for-next

Thanks!

[1/1] regulator: as3722: Fix fall-through warnings for Clang
  commit: b52b417ccac4fae5b1f2ec4f1d46eb91e4493dc5

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v3 05/12] drm/ttm: Expose ttm_tt_unpopulate for driver use

[Andrey Grodzovsky]

On 11/23/20 3:20 PM, Christian König wrote:

Am 23.11.20 um 21:05 schrieb Andrey Grodzovsky:


On 11/25/20 5:42 AM, Christian König wrote:

Am 21.11.20 um 06:21 schrieb Andrey Grodzovsky:

It's needed to drop iommu backed pages on device unplug
before device's IOMMU group is released.


It would be cleaner if we could do the whole handling in TTM. I also need to 
double check what you are doing with this function.


Christian.



Check patch "drm/amdgpu: Register IOMMU topology notifier per device." to see
how i use it. I don't see why this should go into TTM mid-layer - the stuff I 
do inside

is vendor specific and also I don't think TTM is explicitly aware of IOMMU ?
Do you mean you prefer the IOMMU notifier to be registered from within TTM
and then use a hook to call into vendor specific handler ?


No, that is really vendor specific.

What I meant is to have a function like ttm_resource_manager_evict_all() which 
you only need to call and all tt objects are unpopulated.



So instead of this BO list i create and later iterate in amdgpu from the IOMMU 
patch you just want to do it within

TTM with a single function ? Makes much more sense.

Andrey




Give me a day or two to look into this.

Christian.



Andrey






Signed-off-by: Andrey Grodzovsky 
---
  drivers/gpu/drm/ttm/ttm_tt.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 1ccf1ef..29248a5 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -495,3 +495,4 @@ void ttm_tt_unpopulate(struct ttm_tt *ttm)
  else
  ttm_pool_unpopulate(ttm);
  }
+EXPORT_SYMBOL(ttm_tt_unpopulate);



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=04%7C01%7CAndrey.Grodzovsky%40amd.com%7C9be029f26a4746347a6108d88fed299b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637417596065559955%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=tZ3do%2FeKzBtRlNaFbBjCtRvUHKdvwDZ7SoYhEBu4%2BT8%3Dreserved=0 




___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v3 05/12] drm/ttm: Expose ttm_tt_unpopulate for driver use

[Christian König]

Am 23.11.20 um 21:05 schrieb Andrey Grodzovsky:


On 11/25/20 5:42 AM, Christian König wrote:

Am 21.11.20 um 06:21 schrieb Andrey Grodzovsky:

It's needed to drop iommu backed pages on device unplug
before device's IOMMU group is released.


It would be cleaner if we could do the whole handling in TTM. I also 
need to double check what you are doing with this function.


Christian.



Check patch "drm/amdgpu: Register IOMMU topology notifier per device." 
to see
how i use it. I don't see why this should go into TTM mid-layer - the 
stuff I do inside
is vendor specific and also I don't think TTM is explicitly aware of 
IOMMU ?
Do you mean you prefer the IOMMU notifier to be registered from within 
TTM

and then use a hook to call into vendor specific handler ?


No, that is really vendor specific.

What I meant is to have a function like ttm_resource_manager_evict_all() 
which you only need to call and all tt objects are unpopulated.


Give me a day or two to look into this.

Christian.



Andrey






Signed-off-by: Andrey Grodzovsky 
---
  drivers/gpu/drm/ttm/ttm_tt.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c 
b/drivers/gpu/drm/ttm/ttm_tt.c

index 1ccf1ef..29248a5 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -495,3 +495,4 @@ void ttm_tt_unpopulate(struct ttm_tt *ttm)
  else
  ttm_pool_unpopulate(ttm);
  }
+EXPORT_SYMBOL(ttm_tt_unpopulate);



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: unpack dma_fence_chain containers during sync

[Christian König]

That the CPU round trip is gone now.

Christian.

Am 23.11.20 um 20:49 schrieb Marek Olšák:

What is the behavior we should expect?

Marek

On Mon, Nov 23, 2020 at 7:31 AM Christian König 
> wrote:


Ping, Pierre/Marek does this change works as expected?

Regards,
Christian.

Am 18.11.20 um 14:20 schrieb Christian König:
> This allows for optimizing the CPU round trip away.
>
> Signed-off-by: Christian König mailto:christian.koe...@amd.com>>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c   |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 27

>   drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h |  1 +
>   3 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 79342976fa76..68f9a4adf5d2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -1014,7 +1014,7 @@ static int
amdgpu_syncobj_lookup_and_add_to_sync(struct amdgpu_cs_parser *p,
>               return r;
>       }
>
> -     r = amdgpu_sync_fence(>job->sync, fence);
> +     r = amdgpu_sync_fence_chain(>job->sync, fence);
>       dma_fence_put(fence);
>
>       return r;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> index 8ea6c49529e7..d0d64af06f54 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> @@ -28,6 +28,8 @@
>    *    Christian König mailto:christian.koe...@amd.com>>
>    */
>
> +#include 
> +
>   #include "amdgpu.h"
>   #include "amdgpu_trace.h"
>   #include "amdgpu_amdkfd.h"
> @@ -169,6 +171,31 @@ int amdgpu_sync_fence(struct amdgpu_sync
*sync, struct dma_fence *f)
>       return 0;
>   }
>
> +/**
> + * amdgpu_sync_fence_chain - unpack dma_fence_chain and sync
> + *
> + * @sync: sync object to add fence to
> + * @f: potential dma_fence_chain to sync to.
> + *
> + * Add the fences inside the chain to the sync object.
> + */
> +int amdgpu_sync_fence_chain(struct amdgpu_sync *sync, struct
dma_fence *f)
> +{
> +     int r;
> +
> +     dma_fence_chain_for_each(f, f) {
> +             if (dma_fence_is_signaled(f))
> +                     continue;
> +
> +             r = amdgpu_sync_fence(sync, f);
> +             if (r) {
> +                     dma_fence_put(f);
> +                     return r;
> +             }
> +     }
> +     return 0;
> +}
> +
>   /**
>    * amdgpu_sync_vm_fence - remember to sync to this VM fence
>    *
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
> index 7c0fe20c470d..b142175b65b6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
> @@ -48,6 +48,7 @@ struct amdgpu_sync {
>
>   void amdgpu_sync_create(struct amdgpu_sync *sync);
>   int amdgpu_sync_fence(struct amdgpu_sync *sync, struct
dma_fence *f);
> +int amdgpu_sync_fence_chain(struct amdgpu_sync *sync, struct
dma_fence *f);
>   int amdgpu_sync_vm_fence(struct amdgpu_sync *sync, struct
dma_fence *fence);
>   int amdgpu_sync_resv(struct amdgpu_device *adev, struct
amdgpu_sync *sync,
>                    struct dma_resv *resv, enum amdgpu_sync_mode
mode,

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org 
https://lists.freedesktop.org/mailman/listinfo/amd-gfx



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v3 05/12] drm/ttm: Expose ttm_tt_unpopulate for driver use

[Andrey Grodzovsky]

On 11/25/20 5:42 AM, Christian König wrote:

Am 21.11.20 um 06:21 schrieb Andrey Grodzovsky:

It's needed to drop iommu backed pages on device unplug
before device's IOMMU group is released.


It would be cleaner if we could do the whole handling in TTM. I also need to 
double check what you are doing with this function.


Christian.



Check patch "drm/amdgpu: Register IOMMU topology notifier per device." to see
how i use it. I don't see why this should go into TTM mid-layer - the stuff I do 
inside

is vendor specific and also I don't think TTM is explicitly aware of IOMMU ?
Do you mean you prefer the IOMMU notifier to be registered from within TTM
and then use a hook to call into vendor specific handler ?

Andrey






Signed-off-by: Andrey Grodzovsky 
---
  drivers/gpu/drm/ttm/ttm_tt.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 1ccf1ef..29248a5 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -495,3 +495,4 @@ void ttm_tt_unpopulate(struct ttm_tt *ttm)
  else
  ttm_pool_unpopulate(ttm);
  }
+EXPORT_SYMBOL(ttm_tt_unpopulate);



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[James Bottomley]

On Mon, 2020-11-23 at 15:19 +0100, Miguel Ojeda wrote:
> On Sun, Nov 22, 2020 at 11:36 PM James Bottomley
>  wrote:
> > Well, it seems to be three years of someone's time plus the
> > maintainer review time and series disruption of nearly a thousand
> > patches.  Let's be conservative and assume the producer worked
> > about 30% on the series and it takes about 5-10 minutes per patch
> > to review, merge and for others to rework existing series.  So
> > let's say it's cost a person year of a relatively junior engineer
> > producing the patches and say 100h of review and application
> > time.  The latter is likely the big ticket item because it's what
> > we have in least supply in the kernel (even though it's 20x vs the
> > producer time).
> 
> How are you arriving at such numbers? It is a total of ~200 trivial
> lines.

Well, I used git.  It says that as of today in Linus' tree we have 889
patches related to fall throughs and the first series went in in
october 2017 ... ignoring a couple of outliers back to February.

> > It's not about the risk of the changes it's about the cost of
> > implementing them.  Even if you discount the producer time (which
> > someone gets to pay for, and if I were the engineering manager, I'd
> > be unhappy about), the review/merge/rework time is pretty
> > significant in exchange for six minor bug fixes.  Fine, when a new
> > compiler warning comes along it's certainly reasonable to see if we
> > can benefit from it and the fact that the compiler people think
> > it's worthwhile is enough evidence to assume this initially.  But
> > at some point you have to ask whether that assumption is supported
> > by the evidence we've accumulated over the time we've been using
> > it.  And if the evidence doesn't support it perhaps it is time to
> > stop the experiment.
> 
> Maintainers routinely review 1-line trivial patches, not to mention
> internal API changes, etc.

We're also complaining about the inability to recruit maintainers:

https://www.theregister.com/2020/06/30/hard_to_find_linux_maintainers_says_torvalds/

And burn out:

http://antirez.com/news/129

The whole crux of your argument seems to be maintainers' time isn't
important so we should accept all trivial patches ... I'm pushing back
on that assumption in two places, firstly the valulessness of the time
and secondly that all trivial patches are valuable.

> If some company does not want to pay for that, that's fine, but they
> don't get to be maintainers and claim `Supported`.

What I'm actually trying to articulate is a way of measuring value of
the patch vs cost ... it has nothing really to do with who foots the
actual bill.

One thesis I'm actually starting to formulate is that this continual
devaluing of maintainers is why we have so much difficulty keeping and
recruiting them.

James



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[Joe Perches]

On Mon, 2020-11-23 at 07:58 -0800, James Bottomley wrote:
> We're also complaining about the inability to recruit maintainers:
> 
> https://www.theregister.com/2020/06/30/hard_to_find_linux_maintainers_says_torvalds/
> 
> And burn out:
> 
> http://antirez.com/news/129

https://www.wired.com/story/open-source-coders-few-tired/

> What I'm actually trying to articulate is a way of measuring value of
> the patch vs cost ... it has nothing really to do with who foots the
> actual bill.

It's unclear how to measure value in consistency.

But one way that costs can be reduced is by automation and _not_
involving maintainers when the patch itself is provably correct.

> One thesis I'm actually starting to formulate is that this continual
> devaluing of maintainers is why we have so much difficulty keeping and
> recruiting them.

The linux kernel has something like 1500 different maintainers listed
in the MAINTAINERS file.  That's not a trivial number.

$ git grep '^M:' MAINTAINERS | sort | uniq -c | wc -l
1543
$ git grep '^M:' MAINTAINERS| cut -f1 -d'<' | sort | uniq -c | wc -l
1446

I think the question you are asking is about trust and how it
effects development.

And back to that wired story, the actual number of what you might
be considering to be maintainers is likely less than 10% of the
listed numbers above.


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[Miguel Ojeda]

On Mon, Nov 23, 2020 at 4:58 PM James Bottomley
 wrote:
>
> Well, I used git.  It says that as of today in Linus' tree we have 889
> patches related to fall throughs and the first series went in in
> october 2017 ... ignoring a couple of outliers back to February.

I can see ~10k insertions over ~1k commits and 15 years that mention a
fallthrough in the entire repo. That is including some commits (like
the biggest one, 960 insertions) that have nothing to do with C
fallthrough. A single kernel release has an order of magnitude more
changes than this...

But if we do the math, for an author, at even 1 minute per line change
and assuming nothing can be automated at all, it would take 1 month of
work. For maintainers, a couple of trivial lines is noise compared to
many other patches.

In fact, this discussion probably took more time than the time it
would take to review the 200 lines. :-)

> We're also complaining about the inability to recruit maintainers:
>
> https://www.theregister.com/2020/06/30/hard_to_find_linux_maintainers_says_torvalds/
>
> And burn out:
>
> http://antirez.com/news/129

Accepting trivial and useful 1-line patches is not what makes a
voluntary maintainer quit... Thankless work with demanding deadlines is.

> The whole crux of your argument seems to be maintainers' time isn't
> important so we should accept all trivial patches

I have not said that, at all. In fact, I am a voluntary one and I
welcome patches like this. It takes very little effort on my side to
review and it helps the kernel overall. Paid maintainers are the ones
that can take care of big features/reviews.

> What I'm actually trying to articulate is a way of measuring value of
> the patch vs cost ... it has nothing really to do with who foots the
> actual bill.

I understand your point, but you were the one putting it in terms of a
junior FTE. In my view, 1 month-work (worst case) is very much worth
removing a class of errors from a critical codebase.

> One thesis I'm actually starting to formulate is that this continual
> devaluing of maintainers is why we have so much difficulty keeping and
> recruiting them.

That may very well be true, but I don't feel anybody has devalued
maintainers in this discussion.

Cheers,
Miguel
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang

[James Bottomley]

On Mon, 2020-11-23 at 07:03 -0600, Gustavo A. R. Silva wrote:
> On Sun, Nov 22, 2020 at 11:53:55AM -0800, James Bottomley wrote:
> > On Sun, 2020-11-22 at 11:22 -0800, Joe Perches wrote:
> > > On Sun, 2020-11-22 at 11:12 -0800, James Bottomley wrote:
> > > > On Sun, 2020-11-22 at 10:25 -0800, Joe Perches wrote:
> > > > > On Sun, 2020-11-22 at 10:21 -0800, James Bottomley wrote:
> > > > > > Please tell me our reward for all this effort isn't a
> > > > > > single missing error print.
> > > > > 
> > > > > There were quite literally dozens of logical defects found
> > > > > by the fallthrough additions.  Very few were logging only.
> > > > 
> > > > So can you give us the best examples (or indeed all of them if
> > > > someone is keeping score)?  hopefully this isn't a US election
> > > > situation ...
> > > 
> > > Gustavo?  Are you running for congress now?
> > > 
> > > https://lwn.net/Articles/794944/
> > 
> > That's 21 reported fixes of which about 50% seem to produce no
> > change in code behaviour at all, a quarter seem to have no user
> > visible effect with the remaining quarter producing unexpected
> > errors on obscure configuration parameters, which is why no-one
> > really noticed them before.
> 
> The really important point here is the number of bugs this has
> prevented and will prevent in the future. See an example of this,
> below:
> 
> https://lore.kernel.org/linux-iio/20190813135802.gb27...@kroah.com/

I think this falls into the same category as the other six bugs: it
changes the output/input for parameters but no-one has really noticed,
usually because the command is obscure or the bias effect is minor.

> This work is still relevant, even if the total number of issues/bugs
> we find in the process is zero (which is not the case).

Really, no ... something which produces no improvement has no value at
all ... we really shouldn't be wasting maintainer time with it because
it has a cost to merge.  I'm not sure we understand where the balance
lies in value vs cost to merge but I am confident in the zero value
case.

> "The sucky thing about doing hard work to deploy hardening is that
> the result is totally invisible by definition (things not happening)
> [..]"
> - Dmitry Vyukov

Really, no.  Something that can't be measured at all doesn't exist.

And actually hardening is one of those things you can measure (which I
do have to admit isn't true for everything in the security space) ...
it's number of exploitable bugs found before you did it vs number of
exploitable bugs found after you did it.  Usually hardening eliminates
a class of bug, so the way I've measured hardening before is to go
through the CVE list for the last couple of years for product X, find
all the bugs that are of the class we're looking to eliminate and say
if we had hardened X against this class of bug we'd have eliminated Y%
of the exploits.  It can be quite impressive if Y is a suitably big
number.

James


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] MAINTAINERS tag for cleanup robot

[Tom Rix]

On 11/22/20 10:22 AM, Joe Perches wrote:
> On Sun, 2020-11-22 at 08:33 -0800, Tom Rix wrote:
>> On 11/21/20 9:10 AM, Joe Perches wrote:
>>> On Sat, 2020-11-21 at 08:50 -0800, t...@redhat.com wrote:
 A difficult part of automating commits is composing the subsystem
 preamble in the commit log.  For the ongoing effort of a fixer producing
 one or two fixes a release the use of 'treewide:' does not seem 
 appropriate.

 It would be better if the normal prefix was used.  Unfortunately normal is
 not consistent across the tree.

 So I am looking for comments for adding a new tag to the MAINTAINERS file

D: Commit subsystem prefix

 ex/ for FPGA DFL DRIVERS

D: fpga: dfl:
>>> I'm all for it.  Good luck with the effort.  It's not completely trivial.
>>>
>>> From a decade ago:
>>>
>>> https://lore.kernel.org/lkml/1289919077.28741.50.camel@Joe-Laptop/
>>>
>>> (and that thread started with extra semicolon patches too)
>> Reading the history, how about this.
>>
>> get_maintainer.pl outputs a single prefix, if multiple files have the
>> same prefix it works, if they don't its an error.
>>
>> Another script 'commit_one_file.sh' does the call to get_mainainter.pl
>> to get the prefix and be called by run-clang-tools.py to get the fixer
>> specific message.
> It's not whether the script used is get_maintainer or any other script,
> the question is really if the MAINTAINERS file is the appropriate place
> to store per-subsystem patch specific prefixes.
>
> It is.
>
> Then the question should be how are the forms described and what is the
> inheritance priority.  My preference would be to have a default of
> inherit the parent base and add basename(subsystem dirname).
>
> Commit history seems to have standardized on using colons as the separator
> between the commit prefix and the subject.
>
> A good mechanism to explore how various subsystems have uses prefixes in
> the past might be something like:
>
> $ git log --no-merges --pretty='%s' -  | \
>   perl -n -e 'print substr($_, 0, rindex($_, ":") + 1) . "\n";' | \
>   sort | uniq -c | sort -rn

Thanks, I have shamelessly stolen this line and limited the commits to the 
maintainer.

I will post something once the generation of the prefixes is done.

Tom

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] MAINTAINERS tag for cleanup robot

[Lukas Bulwahn]

On Mon, Nov 23, 2020 at 4:52 PM Jani Nikula  wrote:
>
> On Sat, 21 Nov 2020, James Bottomley  
> wrote:
> > On Sat, 2020-11-21 at 08:50 -0800, t...@redhat.com wrote:
> >> A difficult part of automating commits is composing the subsystem
> >> preamble in the commit log.  For the ongoing effort of a fixer
> >> producing
> >> one or two fixes a release the use of 'treewide:' does not seem
> >> appropriate.
> >>
> >> It would be better if the normal prefix was used.  Unfortunately
> >> normal is
> >> not consistent across the tree.
> >>
> >>
> >>  D: Commit subsystem prefix
> >>
> >> ex/ for FPGA DFL DRIVERS
> >>
> >>  D: fpga: dfl:
> >>
> >
> > I've got to bet this is going to cause more issues than it solves.
>
> Agreed.
>

Tom, this a problem only kernel janitors encounter; all other
developers really do not have that issue. The time spent on creating
the patch is much larger than the amount saved if the commit log
header line prefix would be derived automatically. I believe Julia
Lawall, Arnd Bergmann and Nathan Chancellor as long-term
high-frequency janitors do have already scripted approaches to that
issue. Maybe they simply need to share these scripts with you and you
consolidate them and share with everyone?

Also, making clean-up patches cumbersome has a positive side as well;
maintainers are not swamped with fully automated patch submissions.
There have been some bad experiences with some submitters on that in
the past...

> > SCSI uses scsi: : for drivers but not every driver has a
> > MAINTAINERS entry.  We use either scsi: or scsi: core: for mid layer
> > things, but we're not consistent.  Block uses blk-: for all
> > of it's stuff but almost no s have a MAINTAINERS entry.  So
> > the next thing you're going to cause is an explosion of suggested
> > MAINTAINERs entries.
>
> On the one hand, adoption of new MAINTAINERS entries has been really
> slow. Look at B, C, or P, for instance. On the other hand, if this were
> to get adopted, you'll potentially get conflicting prefixes for patches
> touching multiple files. Then what?
>
> I'm guessing a script looking at git log could come up with better
> suggestions for prefixes via popularity contest than manually maintained
> MAINTAINERS entries. It might not always get it right, but then human
> outsiders aren't going to always get it right either.
>
> Now you'll only need Someone(tm) to write the script. ;)
>
> Something quick like this:
>
> git log --since={1year} --pretty=format:%s --  |\
> grep -v "^\(Merge\|Revert\)" |\
> sed 's/:[^:]*$//' |\
> sort | uniq -c | sort -rn | head -5
>
> already gives me results that really aren't worse than some of the
> prefixes invented by drive-by contributors.
>

I agree I do not see the need to introduce something in MAINTAINERS;
from my observations maintaining MAINTAINERS, there is sufficient work
on adoption and maintenance of the existing entries already without
such an yet another additional entry. Some entries are outdated or
wrong and the janitor task of cleaning those up is already enough work
for involved janitors and enough churn for involved maintainers. So a
machine-learned approach as above is probably good enough, but if you
think you need more complex rules try to learn them from the data at
hand... certainly a nice task to do with machine learning on commit
message prefixes.

Lukas
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: unpack dma_fence_chain containers during sync

[Marek Olšák]

What is the behavior we should expect?

Marek

On Mon, Nov 23, 2020 at 7:31 AM Christian König <
ckoenig.leichtzumer...@gmail.com> wrote:

> Ping, Pierre/Marek does this change works as expected?
>
> Regards,
> Christian.
>
> Am 18.11.20 um 14:20 schrieb Christian König:
> > This allows for optimizing the CPU round trip away.
> >
> > Signed-off-by: Christian König 
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c   |  2 +-
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 27 
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h |  1 +
> >   3 files changed, 29 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > index 79342976fa76..68f9a4adf5d2 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > @@ -1014,7 +1014,7 @@ static int
> amdgpu_syncobj_lookup_and_add_to_sync(struct amdgpu_cs_parser *p,
> >   return r;
> >   }
> >
> > - r = amdgpu_sync_fence(>job->sync, fence);
> > + r = amdgpu_sync_fence_chain(>job->sync, fence);
> >   dma_fence_put(fence);
> >
> >   return r;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> > index 8ea6c49529e7..d0d64af06f54 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
> > @@ -28,6 +28,8 @@
> >*Christian König 
> >*/
> >
> > +#include 
> > +
> >   #include "amdgpu.h"
> >   #include "amdgpu_trace.h"
> >   #include "amdgpu_amdkfd.h"
> > @@ -169,6 +171,31 @@ int amdgpu_sync_fence(struct amdgpu_sync *sync,
> struct dma_fence *f)
> >   return 0;
> >   }
> >
> > +/**
> > + * amdgpu_sync_fence_chain - unpack dma_fence_chain and sync
> > + *
> > + * @sync: sync object to add fence to
> > + * @f: potential dma_fence_chain to sync to.
> > + *
> > + * Add the fences inside the chain to the sync object.
> > + */
> > +int amdgpu_sync_fence_chain(struct amdgpu_sync *sync, struct dma_fence
> *f)
> > +{
> > + int r;
> > +
> > + dma_fence_chain_for_each(f, f) {
> > + if (dma_fence_is_signaled(f))
> > + continue;
> > +
> > + r = amdgpu_sync_fence(sync, f);
> > + if (r) {
> > + dma_fence_put(f);
> > + return r;
> > + }
> > + }
> > + return 0;
> > +}
> > +
> >   /**
> >* amdgpu_sync_vm_fence - remember to sync to this VM fence
> >*
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
> > index 7c0fe20c470d..b142175b65b6 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
> > @@ -48,6 +48,7 @@ struct amdgpu_sync {
> >
> >   void amdgpu_sync_create(struct amdgpu_sync *sync);
> >   int amdgpu_sync_fence(struct amdgpu_sync *sync, struct dma_fence *f);
> > +int amdgpu_sync_fence_chain(struct amdgpu_sync *sync, struct dma_fence
> *f);
> >   int amdgpu_sync_vm_fence(struct amdgpu_sync *sync, struct dma_fence
> *fence);
> >   int amdgpu_sync_resv(struct amdgpu_device *adev, struct amdgpu_sync
> *sync,
> >struct dma_resv *resv, enum amdgpu_sync_mode mode,
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu/dce_virtual: Enable vBlank control for vf

[shaoyunl]

This function actually control the vblank on/off. It shouldn't be bypassed 
for VF. Otherwise all the vblank based feature on VF will not work.

Signed-off-by: shaoyunl 
Change-Id: I77c6f57bb0af390b61f0049c12bf425b10d70d91
---
 drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c 
b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
index b4d4b76538d2..ffcc64ec6473 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
@@ -139,9 +139,6 @@ static void dce_virtual_crtc_dpms(struct drm_crtc *crtc, 
int mode)
struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
unsigned type;
 
-   if (amdgpu_sriov_vf(adev))
-   return;
-
switch (mode) {
case DRM_MODE_DPMS_ON:
amdgpu_crtc->enabled = true;
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amdgpu/dce_virtual: Enable DPM for vf

[Liu, Shaoyun]

[AMD Official Use Only - Internal Distribution Only]

Sorry , please ignore this one , I change the subject as suggested and resent 
the  patch

Shaoyun.liu

-Original Message- 
From: Liu, Shaoyun  
Sent: Monday, November 23, 2020 12:24 PM
To: amd-gfx@lists.freedesktop.org
Cc: Liu, Shaoyun 
Subject: [PATCH] drm/amdgpu/dce_virtual: Enable DPM for vf

This function actually control the vblank on/off. It shouldn't be bypassed 
for VF. Otherwise all the vblank based feature on VF will not work.

Signed-off-by: shaoyunl 
Change-Id: I77c6f57bb0af390b61f0049c12bf425b10d70d91
---
 drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c 
b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
index b4d4b76538d2..ffcc64ec6473 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
@@ -139,9 +139,6 @@ static void dce_virtual_crtc_dpms(struct drm_crtc *crtc, 
int mode)
struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
unsigned type;
 
-   if (amdgpu_sriov_vf(adev))
-   return;
-
switch (mode) {
case DRM_MODE_DPMS_ON:
amdgpu_crtc->enabled = true;
-- 
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu/dce_virtual: Enable DPM for vf

[shaoyunl]

This function actually control the vblank on/off. It shouldn't be bypassed 
for VF. Otherwise all the vblank based feature on VF will not work.

Signed-off-by: shaoyunl 
Change-Id: I77c6f57bb0af390b61f0049c12bf425b10d70d91
---
 drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c 
b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
index b4d4b76538d2..ffcc64ec6473 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
@@ -139,9 +139,6 @@ static void dce_virtual_crtc_dpms(struct drm_crtc *crtc, 
int mode)
struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
unsigned type;
 
-   if (amdgpu_sriov_vf(adev))
-   return;
-
switch (mode) {
case DRM_MODE_DPMS_ON:
amdgpu_crtc->enabled = true;
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu/dce_virtual: Enable DPM for vf

[Michel Dänzer]

On 2020-11-23 5:40 p.m., shaoyunl wrote:

 This function actually control the vblank on/off. It shouldn't be bypassed 
for VF. Otherwise all the vblank based feature on VF will not work.

Signed-off-by: shaoyunl 
Change-Id: I77c6f57bb0af390b61f0049c12bf425b10d70d91
---
  drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 3 ---
  1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c 
b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
index b4d4b76538d2..ffcc64ec6473 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
@@ -139,9 +139,6 @@ static void dce_virtual_crtc_dpms(struct drm_crtc *crtc, 
int mode)
struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
unsigned type;
  
-	if (amdgpu_sriov_vf(adev))

-   return;
-
switch (mode) {
case DRM_MODE_DPMS_ON:
amdgpu_crtc->enabled = true;



Subject says DPM, but looks like this is about DPMS?


--
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amdgpu/dce_virtual: Enable DPM for vf

[Zhang, Hawking]

[AMD Public Use]

Reviewed-by: Hawking Zhang 

Regards,
Hawking
-Original Message-
From: amd-gfx  On Behalf Of shaoyunl
Sent: Tuesday, November 24, 2020 00:41
To: amd-gfx@lists.freedesktop.org
Cc: Liu, Shaoyun 
Subject: [PATCH] drm/amdgpu/dce_virtual: Enable DPM for vf

This function actually control the vblank on/off. It shouldn't be bypassed 
for VF. Otherwise all the vblank based feature on VF will not work.

Signed-off-by: shaoyunl 
Change-Id: I77c6f57bb0af390b61f0049c12bf425b10d70d91
---
 drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c 
b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
index b4d4b76538d2..ffcc64ec6473 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
@@ -139,9 +139,6 @@ static void dce_virtual_crtc_dpms(struct drm_crtc *crtc, 
int mode)
struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
unsigned type;
 
-   if (amdgpu_sriov_vf(adev))
-   return;
-
switch (mode) {
case DRM_MODE_DPMS_ON:
amdgpu_crtc->enabled = true;
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=04%7C01%7Chawking.zhang%40amd.com%7Cc1c948ca8d4b478f3c5f08d88fceab9b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637417465126174440%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=VlFBXGWwjYN49ufyahlQrS%2Fnm%2FFojwp%2Fp7S8DRpB2kk%3Dreserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu/dce_virtual: Enable DPM for vf

[shaoyunl]

This function actually control the vblank on/off. It shouldn't be bypassed 
for VF. Otherwise all the vblank based feature on VF will not work.

Signed-off-by: shaoyunl 
Change-Id: I77c6f57bb0af390b61f0049c12bf425b10d70d91
---
 drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c 
b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
index b4d4b76538d2..ffcc64ec6473 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
@@ -139,9 +139,6 @@ static void dce_virtual_crtc_dpms(struct drm_crtc *crtc, 
int mode)
struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
unsigned type;
 
-   if (amdgpu_sriov_vf(adev))
-   return;
-
switch (mode) {
case DRM_MODE_DPMS_ON:
amdgpu_crtc->enabled = true;
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[Rafael J. Wysocki]

On Mon, Nov 23, 2020 at 4:58 PM James Bottomley
 wrote:
>
> On Mon, 2020-11-23 at 15:19 +0100, Miguel Ojeda wrote:
> > On Sun, Nov 22, 2020 at 11:36 PM James Bottomley
> >  wrote:

[cut]

> >
> > Maintainers routinely review 1-line trivial patches, not to mention
> > internal API changes, etc.
>
> We're also complaining about the inability to recruit maintainers:
>
> https://www.theregister.com/2020/06/30/hard_to_find_linux_maintainers_says_torvalds/
>
> And burn out:
>
> http://antirez.com/news/129

Right.

> The whole crux of your argument seems to be maintainers' time isn't
> important so we should accept all trivial patches ... I'm pushing back
> on that assumption in two places, firstly the valulessness of the time
> and secondly that all trivial patches are valuable.
>
> > If some company does not want to pay for that, that's fine, but they
> > don't get to be maintainers and claim `Supported`.
>
> What I'm actually trying to articulate is a way of measuring value of
> the patch vs cost ... it has nothing really to do with who foots the
> actual bill.
>
> One thesis I'm actually starting to formulate is that this continual
> devaluing of maintainers is why we have so much difficulty keeping and
> recruiting them.

Absolutely.

This is just one of the factors involved, but a significant one IMV.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] MAINTAINERS tag for cleanup robot

[Jani Nikula]

On Sat, 21 Nov 2020, James Bottomley  
wrote:
> On Sat, 2020-11-21 at 08:50 -0800, t...@redhat.com wrote:
>> A difficult part of automating commits is composing the subsystem
>> preamble in the commit log.  For the ongoing effort of a fixer
>> producing
>> one or two fixes a release the use of 'treewide:' does not seem
>> appropriate.
>> 
>> It would be better if the normal prefix was used.  Unfortunately
>> normal is
>> not consistent across the tree.
>> 
>> 
>>  D: Commit subsystem prefix
>> 
>> ex/ for FPGA DFL DRIVERS
>> 
>>  D: fpga: dfl:
>> 
>
> I've got to bet this is going to cause more issues than it solves.

Agreed.

> SCSI uses scsi: : for drivers but not every driver has a
> MAINTAINERS entry.  We use either scsi: or scsi: core: for mid layer
> things, but we're not consistent.  Block uses blk-: for all
> of it's stuff but almost no s have a MAINTAINERS entry.  So
> the next thing you're going to cause is an explosion of suggested
> MAINTAINERs entries.

On the one hand, adoption of new MAINTAINERS entries has been really
slow. Look at B, C, or P, for instance. On the other hand, if this were
to get adopted, you'll potentially get conflicting prefixes for patches
touching multiple files. Then what?

I'm guessing a script looking at git log could come up with better
suggestions for prefixes via popularity contest than manually maintained
MAINTAINERS entries. It might not always get it right, but then human
outsiders aren't going to always get it right either.

Now you'll only need Someone(tm) to write the script. ;)

Something quick like this:

git log --since={1year} --pretty=format:%s --  |\
grep -v "^\(Merge\|Revert\)" |\
sed 's/:[^:]*$//' |\
sort | uniq -c | sort -rn | head -5

already gives me results that really aren't worse than some of the
prefixes invented by drive-by contributors.

> Has anyone actually complained about treewide:?

As Joe said, I'd feel silly applying patches to drivers with that
prefix. If it gets applied by someone else higher up, literally
treewide, then no complaints.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[Miguel Ojeda]

On Sun, Nov 22, 2020 at 11:36 PM James Bottomley
 wrote:
>
> Well, it seems to be three years of someone's time plus the maintainer
> review time and series disruption of nearly a thousand patches.  Let's
> be conservative and assume the producer worked about 30% on the series
> and it takes about 5-10 minutes per patch to review, merge and for
> others to rework existing series.  So let's say it's cost a person year
> of a relatively junior engineer producing the patches and say 100h of
> review and application time.  The latter is likely the big ticket item
> because it's what we have in least supply in the kernel (even though
> it's 20x vs the producer time).

How are you arriving at such numbers? It is a total of ~200 trivial lines.

> It's not about the risk of the changes it's about the cost of
> implementing them.  Even if you discount the producer time (which
> someone gets to pay for, and if I were the engineering manager, I'd be
> unhappy about), the review/merge/rework time is pretty significant in
> exchange for six minor bug fixes.  Fine, when a new compiler warning
> comes along it's certainly reasonable to see if we can benefit from it
> and the fact that the compiler people think it's worthwhile is enough
> evidence to assume this initially.  But at some point you have to ask
> whether that assumption is supported by the evidence we've accumulated
> over the time we've been using it.  And if the evidence doesn't support
> it perhaps it is time to stop the experiment.

Maintainers routinely review 1-line trivial patches, not to mention
internal API changes, etc.

If some company does not want to pay for that, that's fine, but they
don't get to be maintainers and claim `Supported`.

Cheers,
Miguel
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[Miguel Ojeda]

On Sun, Nov 22, 2020 at 11:54 PM Finn Thain  wrote:
>
> We should also take into account optimisim about future improvements in
> tooling.

Not sure what you mean here. There is no reliable way to guess what
the intention was with a missing fallthrough, even if you parsed
whitespace and indentation.

> It is if you want to spin it that way.

How is that a "spin"? It is a fact that we won't get *implicit*
fallthrough mistakes anymore (in particular if we make it a hard
error).

> But what we inevitably get is changes like this:
>
>  case 3:
> this();
> +   break;
>  case 4:
> hmmm();
>
> Why? Mainly to silence the compiler. Also because the patch author argued
> successfully that they had found a theoretical bug, often in mature code.

If someone changes control flow, that is on them. Every kernel
developer knows what `break` does.

> But is anyone keeping score of the regressions? If unreported bugs count,
> what about unreported regressions?

Introducing `fallthrough` does not change semantics. If you are really
keen, you can always compare the objects because the generated code
shouldn't change.

Cheers,
Miguel
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 22/40] drm/amd/amdgpu/dce_v6_0: Fix formatting and missing parameter description issues

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c:192: warning: Function parameter or 
member 'async' not described in 'dce_v6_0_page_flip'
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c:1050: warning: Cannot understand  *

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Luben Tuikov 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c 
b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index 9439763493464..83a88385b7620 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -180,6 +180,7 @@ static void dce_v6_0_pageflip_interrupt_fini(struct 
amdgpu_device *adev)
  * @adev: amdgpu_device pointer
  * @crtc_id: crtc to cleanup pageflip on
  * @crtc_base: new address of the crtc (GPU MC address)
+ * @async: asynchronous flip
  *
  * Does the actual pageflip (evergreen+).
  * During vblank we take the crtc lock and wait for the update_pending
@@ -1047,7 +1048,6 @@ static u32 dce_v6_0_line_buffer_adjust(struct 
amdgpu_device *adev,
 
 
 /**
- *
  * dce_v6_0_bandwidth_update - program display watermarks
  *
  * @adev: amdgpu_device pointer
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 00/40] [Set 8] Rid W=1 warnings from GPU

[Lee Jones]

This set is part of a larger effort attempting to clean-up W=1
kernel builds, which are currently overwhelmingly riddled with
niggly little warnings.

Only 900 (from 5000) to go!

Lee Jones (40):
  drm/radeon/radeon_device: Consume our own header where the prototypes
are located
  drm/amd/amdgpu/amdgpu_ttm: Add description for 'page_flags'
  drm/amd/amdgpu/amdgpu_ib: Provide docs for 'amdgpu_ib_schedule()'s
'job' param
  drm/amd/amdgpu/amdgpu_virt: Correct possible copy/paste or doc-rot
misnaming issue
  drm/amd/amdgpu/cik_ih: Supply description for 'ih' in
'cik_ih_{get,set}_wptr()'
  drm/amd/amdgpu/uvd_v4_2: Fix some kernel-doc misdemeanours
  drm/amd/amdgpu/dce_v8_0: Supply description for 'async'
  drm/amd/amdgpu/cik_sdma: Supply some missing function param
descriptions
  drm/amd/amdgpu/gfx_v7_0: Clean-up a bunch of kernel-doc related issues
  drm/msm/disp/dpu1/dpu_core_perf: Fix kernel-doc formatting issues
  drm/msm/disp/dpu1/dpu_hw_blk: Add one missing and remove an extra
param description
  drm/msm/disp/dpu1/dpu_formats: Demote non-conformant kernel-doc header
  drm/msm/disp/dpu1/dpu_hw_catalog: Remove duplicated initialisation of
'max_linewidth'
  drm/msm/disp/dpu1/dpu_hw_catalog: Move definitions to the only place
they are used
  drm/nouveau/nvkm/subdev/bios/init: Demote obvious abuse of kernel-doc
  drm/amd/amdgpu/si_dma: Fix a bunch of function documentation issues
  drm/amd/amdgpu/gfx_v6_0: Supply description for
'gfx_v6_0_ring_test_ib()'s 'timeout' param
  drm/msm/disp/dpu1/dpu_encoder: Fix a few parameter/member formatting
issues
  drm/msm/disp/dpu1/dpu_hw_lm: Fix misnaming of parameter 'ctx'
  drm/msm/disp/dpu1/dpu_hw_sspp: Fix kernel-doc formatting abuse
  drm/amd/amdgpu/uvd_v3_1: Fix-up some documentation issues
  drm/amd/amdgpu/dce_v6_0: Fix formatting and missing parameter
description issues
  drm/amd/include/vega20_ip_offset: Mark top-level IP_BASE definition as
__maybe_unused
  drm/amd/include/navi10_ip_offset: Mark top-level IP_BASE as
__maybe_unused
  drm/amd/include/arct_ip_offset: Mark top-level IP_BASE definition as
__maybe_unused
  drm/amd/include/navi14_ip_offset: Mark top-level IP_BASE as
__maybe_unused
  drm/amd/include/navi12_ip_offset: Mark top-level IP_BASE as
__maybe_unused
  drm/amd/include/sienna_cichlid_ip_offset: Mark top-level IP_BASE as
__maybe_unused
  drm/amd/include/vangogh_ip_offset: Mark top-level IP_BASE as
__maybe_unused
  drm/amd/include/dimgrey_cavefish_ip_offset: Mark top-level IP_BASE as
__maybe_unused
  drm/msm/disp/dpu1/dpu_rm: Fix formatting issues and supply
'global_state' description
  drm/msm/disp/dpu1/dpu_vbif: Fix a couple of function param
descriptions
  drm/amd/amdgpu/cik_sdma: Add one and remove another function param
description
  drm/amd/amdgpu/uvd_v4_2: Add one and remove another function param
description
  drm/msm/disp/dpu1/dpu_plane: Fix some spelling and missing function
param descriptions
  drm/amd/amdgpu/gmc_v7_0: Add some missing kernel-doc descriptions
  drm/amd/amdgpu/gmc_v8_0: Fix more issues attributed to copy/paste
  drm/msm/msm_drv: Make '_msm_ioremap()' static
  drm/amd/amdgpu/gmc_v9_0: Remove unused table
'ecc_umc_mcumc_status_addrs'
  drm/amd/amdgpu/gmc_v9_0: Suppy some missing function doc descriptions

 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c|   1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c   |   1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c  |  12 +-
 drivers/gpu/drm/amd/amdgpu/cik_ih.c   |   2 +
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c |  18 ++-
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c |   2 +-
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c |   1 +
 drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c |   1 +
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c |  33 +++--
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c |   7 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c |   5 +
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c |  38 +
 drivers/gpu/drm/amd/amdgpu/si_dma.c   |  14 +-
 drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c |  10 +-
 drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c |  10 +-
 drivers/gpu/drm/amd/include/arct_ip_offset.h  |   4 +-
 .../amd/include/dimgrey_cavefish_ip_offset.h  |   2 +-
 .../gpu/drm/amd/include/navi10_ip_offset.h|   2 +-
 .../gpu/drm/amd/include/navi12_ip_offset.h|   2 +-
 .../gpu/drm/amd/include/navi14_ip_offset.h|   2 +-
 .../amd/include/sienna_cichlid_ip_offset.h|   2 +-
 .../gpu/drm/amd/include/vangogh_ip_offset.h   |   2 +-
 .../gpu/drm/amd/include/vega20_ip_offset.h|   2 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c |  17 +--
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   |  15 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c   |   2 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_blk.c|   2 +-
 .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c|  75 +-
 .../drm/msm/disp/dpu1/dpu_hw_catalog_format.h |  88 
 

[PATCH 08/40] drm/amd/amdgpu/cik_sdma: Supply some missing function param descriptions

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/amd/amdgpu/cik_sdma.c:226: warning: Function parameter or 
member 'job' not described in 'cik_sdma_ring_emit_ib'
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c:226: warning: Function parameter or 
member 'flags' not described in 'cik_sdma_ring_emit_ib'
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c:278: warning: Function parameter or 
member 'addr' not described in 'cik_sdma_ring_emit_fence'
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c:278: warning: Function parameter or 
member 'seq' not described in 'cik_sdma_ring_emit_fence'
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c:278: warning: Function parameter or 
member 'flags' not described in 'cik_sdma_ring_emit_fence'
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c:278: warning: Excess function parameter 
'fence' description in 'cik_sdma_ring_emit_fence'
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c:663: warning: Function parameter or 
member 'timeout' not described in 'cik_sdma_ring_test_ib'
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c:808: warning: Function parameter or 
member 'ring' not described in 'cik_sdma_ring_pad_ib'
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c:859: warning: Function parameter or 
member 'vmid' not described in 'cik_sdma_ring_emit_vm_flush'
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c:859: warning: Function parameter or 
member 'pd_addr' not described in 'cik_sdma_ring_emit_vm_flush'
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c:859: warning: Excess function parameter 
'vm' description in 'cik_sdma_ring_emit_vm_flush'
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c:1315: warning: Function parameter or 
member 'ib' not described in 'cik_sdma_emit_copy_buffer'
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c:1315: warning: Function parameter or 
member 'tmz' not described in 'cik_sdma_emit_copy_buffer'
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c:1315: warning: Excess function parameter 
'ring' description in 'cik_sdma_emit_copy_buffer'
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c:1339: warning: Function parameter or 
member 'ib' not described in 'cik_sdma_emit_fill_buffer'
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c:1339: warning: Excess function parameter 
'ring' description in 'cik_sdma_emit_fill_buffer'

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Sumit Semwal 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Cc: linux-me...@vger.kernel.org
Cc: linaro-mm-...@lists.linaro.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c 
b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
index 1a6494ea50912..f1e9966e7244e 100644
--- a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
@@ -215,7 +215,9 @@ static void cik_sdma_ring_insert_nop(struct amdgpu_ring 
*ring, uint32_t count)
  * cik_sdma_ring_emit_ib - Schedule an IB on the DMA engine
  *
  * @ring: amdgpu ring pointer
+ * @job: job to retrive vmid from
  * @ib: IB object to schedule
+ * @flags: unused
  *
  * Schedule an IB in the DMA ring (CIK).
  */
@@ -267,6 +269,8 @@ static void cik_sdma_ring_emit_hdp_flush(struct amdgpu_ring 
*ring)
  * cik_sdma_ring_emit_fence - emit a fence on the DMA ring
  *
  * @ring: amdgpu ring pointer
+ * @addr: address
+ * @seq: sequence number
  * @fence: amdgpu fence object
  *
  * Add a DMA fence packet to the ring to write
@@ -655,6 +659,7 @@ static int cik_sdma_ring_test_ring(struct amdgpu_ring *ring)
  * cik_sdma_ring_test_ib - test an IB on the DMA engine
  *
  * @ring: amdgpu_ring structure holding ring information
+ * @timeout: timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
  *
  * Test a simple IB in the DMA ring (CIK).
  * Returns 0 on success, error on failure.
@@ -801,6 +806,7 @@ static void cik_sdma_vm_set_pte_pde(struct amdgpu_ib *ib, 
uint64_t pe,
 /**
  * cik_sdma_vm_pad_ib - pad the IB to the required number of dw
  *
+ * @ring: amdgpu_ring structure holding ring information
  * @ib: indirect buffer to fill with padding
  *
  */
@@ -849,7 +855,8 @@ static void cik_sdma_ring_emit_pipeline_sync(struct 
amdgpu_ring *ring)
  * cik_sdma_ring_emit_vm_flush - cik vm flush using sDMA
  *
  * @ring: amdgpu_ring pointer
- * @vm: amdgpu_vm pointer
+ * @vmid: vmid number to use
+ * @pd_addr: address
  *
  * Update the page table base and flush the VM TLB
  * using sDMA (CIK).
@@ -1298,10 +1305,11 @@ static void cik_sdma_set_irq_funcs(struct amdgpu_device 
*adev)
 /**
  * cik_sdma_emit_copy_buffer - copy buffer using the sDMA engine
  *
- * @ring: amdgpu_ring structure holding ring information
+ * @ib: indirect buffer to copy to
  * @src_offset: src GPU address
  * @dst_offset: dst GPU address
  * @byte_count: number of bytes to xfer
+ * @tmz: unused
  *
  * Copy GPU buffers using the DMA engine (CIK).
  * Used by the amdgpu ttm implementation to move pages if
@@ -1325,7 +1333,7 @@ static void cik_sdma_emit_copy_buffer(struct amdgpu_ib 
*ib,
 /**
  * 

[PATCH 04/40] drm/amd/amdgpu/amdgpu_virt: Correct possible copy/paste or doc-rot misnaming issue

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:115: warning: Function parameter or 
member 'adev' not described in 'amdgpu_virt_request_full_gpu'
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:115: warning: Excess function 
parameter 'amdgpu' description in 'amdgpu_virt_request_full_gpu'
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:138: warning: Function parameter or 
member 'adev' not described in 'amdgpu_virt_release_full_gpu'
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:138: warning: Excess function 
parameter 'amdgpu' description in 'amdgpu_virt_release_full_gpu'
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:159: warning: Function parameter or 
member 'adev' not described in 'amdgpu_virt_reset_gpu'
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:159: warning: Excess function 
parameter 'amdgpu' description in 'amdgpu_virt_reset_gpu'
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:194: warning: Function parameter or 
member 'adev' not described in 'amdgpu_virt_wait_reset'
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:194: warning: Excess function 
parameter 'amdgpu' description in 'amdgpu_virt_wait_reset'
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:210: warning: Function parameter or 
member 'adev' not described in 'amdgpu_virt_alloc_mm_table'
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:210: warning: Excess function 
parameter 'amdgpu' description in 'amdgpu_virt_alloc_mm_table'
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:239: warning: Function parameter or 
member 'adev' not described in 'amdgpu_virt_free_mm_table'
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:239: warning: Excess function 
parameter 'amdgpu' description in 'amdgpu_virt_free_mm_table'

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
index 905b85391e64a..462c5dd8ca72c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
@@ -106,7 +106,7 @@ void amdgpu_virt_kiq_reg_write_reg_wait(struct 
amdgpu_device *adev,
 
 /**
  * amdgpu_virt_request_full_gpu() - request full gpu access
- * @amdgpu:amdgpu device.
+ * @adev:  amdgpu device.
  * @init:  is driver init time.
  * When start to init/fini driver, first need to request full gpu access.
  * Return: Zero if request success, otherwise will return error.
@@ -129,7 +129,7 @@ int amdgpu_virt_request_full_gpu(struct amdgpu_device 
*adev, bool init)
 
 /**
  * amdgpu_virt_release_full_gpu() - release full gpu access
- * @amdgpu:amdgpu device.
+ * @adev:  amdgpu device.
  * @init:  is driver init time.
  * When finishing driver init/fini, need to release full gpu access.
  * Return: Zero if release success, otherwise will returen error.
@@ -151,7 +151,7 @@ int amdgpu_virt_release_full_gpu(struct amdgpu_device 
*adev, bool init)
 
 /**
  * amdgpu_virt_reset_gpu() - reset gpu
- * @amdgpu:amdgpu device.
+ * @adev:  amdgpu device.
  * Send reset command to GPU hypervisor to reset GPU that VM is using
  * Return: Zero if reset success, otherwise will return error.
  */
@@ -186,7 +186,7 @@ void amdgpu_virt_request_init_data(struct amdgpu_device 
*adev)
 
 /**
  * amdgpu_virt_wait_reset() - wait for reset gpu completed
- * @amdgpu:amdgpu device.
+ * @adev:  amdgpu device.
  * Wait for GPU reset completed.
  * Return: Zero if reset success, otherwise will return error.
  */
@@ -202,7 +202,7 @@ int amdgpu_virt_wait_reset(struct amdgpu_device *adev)
 
 /**
  * amdgpu_virt_alloc_mm_table() - alloc memory for mm table
- * @amdgpu:amdgpu device.
+ * @adev:  amdgpu device.
  * MM table is used by UVD and VCE for its initialization
  * Return: Zero if allocate success.
  */
@@ -232,7 +232,7 @@ int amdgpu_virt_alloc_mm_table(struct amdgpu_device *adev)
 
 /**
  * amdgpu_virt_free_mm_table() - free mm table memory
- * @amdgpu:amdgpu device.
+ * @adev:  amdgpu device.
  * Free MM table memory
  */
 void amdgpu_virt_free_mm_table(struct amdgpu_device *adev)
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 03/40] drm/amd/amdgpu/amdgpu_ib: Provide docs for 'amdgpu_ib_schedule()'s 'job' param

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c:127: warning: Function parameter or 
member 'job' not described in 'amdgpu_ib_schedule'

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Sumit Semwal 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Cc: linux-me...@vger.kernel.org
Cc: linaro-mm-...@lists.linaro.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index c69af9b86cc60..024d0a563a652 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -106,6 +106,7 @@ void amdgpu_ib_free(struct amdgpu_device *adev, struct 
amdgpu_ib *ib,
  * @ring: ring index the IB is associated with
  * @num_ibs: number of IBs to schedule
  * @ibs: IB objects to schedule
+ * @job: job to schedule
  * @f: fence created during this submission
  *
  * Schedule an IB on the associated ring (all asics).
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 34/40] drm/amd/amdgpu/uvd_v4_2: Add one and remove another function param description

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c:448: warning: Function parameter or 
member 'flags' not described in 'uvd_v4_2_ring_emit_fence'
 drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c:448: warning: Excess function parameter 
'fence' description in 'uvd_v4_2_ring_emit_fence'

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c 
b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
index 2c8c35c3bca52..bf3d1c63739b8 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
@@ -439,7 +439,7 @@ static void uvd_v4_2_stop(struct amdgpu_device *adev)
  * @ring: amdgpu_ring pointer
  * @addr: address
  * @seq: sequence number
- * @fence: fence to emit
+ * @flags: fence related flags
  *
  * Write a fence and a trap command to the ring.
  */
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 23/40] drm/amd/include/vega20_ip_offset: Mark top-level IP_BASE definition as __maybe_unused

[Lee Jones]

 In file included from drivers/gpu/drm/amd/amdgpu/vega20_reg_init.c:27:
 drivers/gpu/drm/amd/amdgpu/../include/vega20_ip_offset.h:154:29: warning: 
‘XDMA_BASE’ defined but not used [-Wunused-const-variable=
 154 | static const struct IP_BASE XDMA_BASE ={ { { { 0x3400, 0, 0, 0, 0, 0 
} },
 | ^
 drivers/gpu/drm/amd/amdgpu/../include/vega20_ip_offset.h:63:29: warning: 
‘FUSE_BASE’ defined but not used [-Wunused-const-variable=]
 63 | static const struct IP_BASE FUSE_BASE ={ { { { 0x00017400, 0, 0, 0, 0, 0 
} },
 | ^

Fixes the following W=1 kernel build warning(s):

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/amd/include/vega20_ip_offset.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/include/vega20_ip_offset.h 
b/drivers/gpu/drm/amd/include/vega20_ip_offset.h
index 2a2a9cc8bedb6..1deb68f3d3341 100644
--- a/drivers/gpu/drm/amd/include/vega20_ip_offset.h
+++ b/drivers/gpu/drm/amd/include/vega20_ip_offset.h
@@ -33,7 +33,7 @@ struct IP_BASE_INSTANCE
 struct IP_BASE
 {
 struct IP_BASE_INSTANCE instance[MAX_INSTANCE];
-};
+} __maybe_unused;
 
 
 static const struct IP_BASE ATHUB_BASE={ { { { 0x0C20, 0, 0, 
0, 0, 0 } },
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 05/40] drm/amd/amdgpu/cik_ih: Supply description for 'ih' in 'cik_ih_{get, set}_wptr()'

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/amd/amdgpu/cik_ih.c:189: warning: Function parameter or member 
'ih' not described in 'cik_ih_get_wptr'
 drivers/gpu/drm/amd/amdgpu/cik_ih.c:274: warning: Function parameter or member 
'ih' not described in 'cik_ih_set_rptr'

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Qinglang Miao 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/amd/amdgpu/cik_ih.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/cik_ih.c 
b/drivers/gpu/drm/amd/amdgpu/cik_ih.c
index db953e95f3d27..d3745711d55f9 100644
--- a/drivers/gpu/drm/amd/amdgpu/cik_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/cik_ih.c
@@ -177,6 +177,7 @@ static void cik_ih_irq_disable(struct amdgpu_device *adev)
  * cik_ih_get_wptr - get the IH ring buffer wptr
  *
  * @adev: amdgpu_device pointer
+ * @ih: IH ring buffer to fetch wptr
  *
  * Get the IH ring buffer wptr from either the register
  * or the writeback memory buffer (CIK).  Also check for
@@ -266,6 +267,7 @@ static void cik_ih_decode_iv(struct amdgpu_device *adev,
  * cik_ih_set_rptr - set the IH ring buffer rptr
  *
  * @adev: amdgpu_device pointer
+ * @ih: IH ring buffer to set wptr
  *
  * Set the IH ring buffer rptr.
  */
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 27/40] drm/amd/include/navi12_ip_offset: Mark top-level IP_BASE as __maybe_unused

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 In file included from drivers/gpu/drm/amd/amdgpu/navi12_reg_init.c:27:
 drivers/gpu/drm/amd/amdgpu/../include/navi12_ip_offset.h:179:29: warning: 
‘USB0_BASE’ defined but not used [-Wunused-const-variable=]
 179 | static const struct IP_BASE USB0_BASE ={ { { { 0x0242A800, 0x05B0, 
0, 0, 0 } },
 | ^
 drivers/gpu/drm/amd/amdgpu/../include/navi12_ip_offset.h:172:29: warning: 
‘UMC_BASE’ defined but not used [-Wunused-const-variable=]
 172 | static const struct IP_BASE UMC_BASE ={ { { { 0x00014000, 0x02425800, 0, 
0, 0 } },
 | ^~~~
 drivers/gpu/drm/amd/amdgpu/../include/navi12_ip_offset.h:151:29: warning: 
‘SDMA_BASE’ defined but not used [-Wunused-const-variable=]
 151 | static const struct IP_BASE SDMA_BASE ={ { { { 0x1260, 0xA000, 
0x02402C00, 0, 0 } },
 | ^

NB: Snipped a few of these

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/amd/include/navi12_ip_offset.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/include/navi12_ip_offset.h 
b/drivers/gpu/drm/amd/include/navi12_ip_offset.h
index 6c2cc6296c061..d8fc00478b6a0 100644
--- a/drivers/gpu/drm/amd/include/navi12_ip_offset.h
+++ b/drivers/gpu/drm/amd/include/navi12_ip_offset.h
@@ -33,7 +33,7 @@ struct IP_BASE_INSTANCE
 struct IP_BASE
 {
 struct IP_BASE_INSTANCE instance[MAX_INSTANCE];
-};
+} __maybe_unused;
 
 
 static const struct IP_BASE ATHUB_BASE ={ { { { 0x0C00, 0x02408C00, 0, 0, 
0 } },
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 07/40] drm/amd/amdgpu/dce_v8_0: Supply description for 'async'

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c:185: warning: Function parameter or 
member 'async' not described in 'dce_v8_0_page_flip'

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Luben Tuikov 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c 
b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index 7973183fa335e..224b30214427f 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
@@ -176,6 +176,7 @@ static void dce_v8_0_pageflip_interrupt_fini(struct 
amdgpu_device *adev)
  * @adev: amdgpu_device pointer
  * @crtc_id: crtc to cleanup pageflip on
  * @crtc_base: new address of the crtc (GPU MC address)
+ * @async: asynchronous flip
  *
  * Triggers the actual pageflip by updating the primary
  * surface base address.
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 01/40] drm/radeon/radeon_device: Consume our own header where the prototypes are located

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/radeon/radeon_device.c:637:6: warning: no previous prototype 
for ‘radeon_device_is_virtual’ [-Wmissing-prototypes]

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/radeon/radeon_device.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/radeon/radeon_device.c 
b/drivers/gpu/drm/radeon/radeon_device.c
index 7f384ffe848a7..ad572f965190b 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -42,6 +42,7 @@
 #include 
 #include 
 
+#include "radeon_device.h"
 #include "radeon_reg.h"
 #include "radeon.h"
 #include "atom.h"
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 40/40] drm/amd/amdgpu/gmc_v9_0: Suppy some missing function doc descriptions

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c:382:23: warning: 
‘ecc_umc_mcumc_status_addrs’ defined but not used [-Wunused-const-variable=]
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c:720: warning: Function parameter or 
member 'vmhub' not described in 'gmc_v9_0_flush_gpu_tlb'
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c:836: warning: Function parameter or 
member 'flush_type' not described in 'gmc_v9_0_flush_gpu_tlb_pasid'
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c:836: warning: Function parameter or 
member 'all_hub' not described in 'gmc_v9_0_flush_gpu_tlb_pasid'

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index fbee43b4ba64d..a83743ab3e8bb 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -675,6 +675,7 @@ static bool gmc_v9_0_get_atc_vmid_pasid_mapping_info(struct 
amdgpu_device *adev,
  *
  * @adev: amdgpu_device pointer
  * @vmid: vm instance to flush
+ * @vmhub: vmhub type
  * @flush_type: the flush type
  *
  * Flush the TLB for the requested page table using certain type.
@@ -791,6 +792,8 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device 
*adev, uint32_t vmid,
  *
  * @adev: amdgpu_device pointer
  * @pasid: pasid to be flush
+ * @flush_type: the flush type
+ * @all_hub: Used with PACKET3_INVALIDATE_TLBS_ALL_HUB()
  *
  * Flush the TLB for the requested pasid.
  */
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 02/40] drm/amd/amdgpu/amdgpu_ttm: Add description for 'page_flags'

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c:1214: warning: Function parameter or 
member 'page_flags' not described in 'amdgpu_ttm_tt_create'

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Sumit Semwal 
Cc: Jerome Glisse 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Cc: linux-me...@vger.kernel.org
Cc: linaro-mm-...@lists.linaro.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 5fcdd67e5a913..debbcef961dd5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1199,6 +1199,7 @@ static void amdgpu_ttm_backend_destroy(struct 
ttm_bo_device *bdev,
  * amdgpu_ttm_tt_create - Create a ttm_tt object for a given BO
  *
  * @bo: The buffer object to create a GTT ttm_tt object around
+ * @page_flags: Page flags to be added to the ttm_tt object
  *
  * Called by ttm_tt_create().
  */
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 29/40] drm/amd/include/vangogh_ip_offset: Mark top-level IP_BASE as __maybe_unused

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 In file included from drivers/gpu/drm/amd/amdgpu/vangogh_reg_init.c:28:
 drivers/gpu/drm/amd/amdgpu/../include/vangogh_ip_offset.h:210:29: warning: 
‘USB_BASE’ defined but not used [-Wunused-const-variable=]
 210 | static const struct IP_BASE USB_BASE = { { { { 0x0242A800, 0x05B0, 
0, 0, 0, 0 } },
 | ^~~~
 drivers/gpu/drm/amd/amdgpu/../include/vangogh_ip_offset.h:202:29: warning: 
‘UMC_BASE’ defined but not used [-Wunused-const-variable=]
 202 | static const struct IP_BASE UMC_BASE = { { { { 0x00014000, 0x02425800, 
0, 0, 0, 0 } },
 | ^~~~
 drivers/gpu/drm/amd/amdgpu/../include/vangogh_ip_offset.h:178:29: warning: 
‘PCIE0_BASE’ defined but not used [-Wunused-const-variable=]
 178 | static const struct IP_BASE PCIE0_BASE = { { { { 0x, 0x0014, 
0x0D20, 0x00010400, 0x0241B000, 0x0404 } },
 | ^~
 drivers/gpu/drm/amd/amdgpu/../include/vangogh_ip_offset.h:154:29: warning: 
‘MP2_BASE’ defined but not used [-Wunused-const-variable=]
 154 | static const struct IP_BASE MP2_BASE = { { { { 0x00016400, 0x02400800, 
0x00F4, 0x00F8, 0x00FC, 0 } },
 | ^~~~

NB: Snipped lots of these

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Huang Rui 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/amd/include/vangogh_ip_offset.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/include/vangogh_ip_offset.h 
b/drivers/gpu/drm/amd/include/vangogh_ip_offset.h
index 2875574b060e6..691073ed780ec 100644
--- a/drivers/gpu/drm/amd/include/vangogh_ip_offset.h
+++ b/drivers/gpu/drm/amd/include/vangogh_ip_offset.h
@@ -36,7 +36,7 @@ struct IP_BASE_INSTANCE
 struct IP_BASE
 {
 struct IP_BASE_INSTANCE instance[MAX_INSTANCE];
-};
+} __maybe_unused;
 
 
 static const struct IP_BASE ACP_BASE = { { { { 0x02403800, 0x0048, 0, 0, 
0, 0 } },
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 17/40] drm/amd/amdgpu/gfx_v6_0: Supply description for 'gfx_v6_0_ring_test_ib()'s 'timeout' param

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c:1903: warning: Function parameter or 
member 'timeout' not described in 'gfx_v6_0_ring_test_ib'

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Sumit Semwal 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Cc: linux-me...@vger.kernel.org
Cc: linaro-mm-...@lists.linaro.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
index 671c46ebeced9..ca74638dec9b7 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
@@ -1894,6 +1894,7 @@ static void gfx_v6_0_ring_emit_ib(struct amdgpu_ring 
*ring,
  * gfx_v6_0_ring_test_ib - basic ring IB test
  *
  * @ring: amdgpu_ring structure holding ring information
+ * @timeout: timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
  *
  * Allocate an IB and execute it on the gfx ring (SI).
  * Provides a basic gfx ring test to verify that IBs are working.
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 00/40] [Set 8] Rid W=1 warnings from GPU

[Lee Jones]

On Mon, 23 Nov 2020, Christian König wrote:

> Only skimmed over them, but over all looks sane to me.
> 
> Series is Acked-by: Christian König 

Thanks Christian, much appreciated.

> Am 23.11.20 um 12:18 schrieb Lee Jones:
> > This set is part of a larger effort attempting to clean-up W=1
> > kernel builds, which are currently overwhelmingly riddled with
> > niggly little warnings.
> > 
> > Only 900 (from 5000) to go!
> > 
> > Lee Jones (40):
> >drm/radeon/radeon_device: Consume our own header where the prototypes
> >  are located
> >drm/amd/amdgpu/amdgpu_ttm: Add description for 'page_flags'
> >drm/amd/amdgpu/amdgpu_ib: Provide docs for 'amdgpu_ib_schedule()'s
> >  'job' param
> >drm/amd/amdgpu/amdgpu_virt: Correct possible copy/paste or doc-rot
> >  misnaming issue
> >drm/amd/amdgpu/cik_ih: Supply description for 'ih' in

[...]

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 26/40] drm/amd/include/navi14_ip_offset: Mark top-level IP_BASE as __maybe_unused

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 In file included from drivers/gpu/drm/amd/amdgpu/navi14_reg_init.c:27:
 drivers/gpu/drm/amd/amdgpu/../include/navi14_ip_offset.h:179:29: warning: 
‘USB0_BASE’ defined but not used [-Wunused-const-variable=]
 179 | static const struct IP_BASE USB0_BASE ={ { { { 0x0242A800, 0x05B0, 
0, 0, 0 } },
 | ^
 drivers/gpu/drm/amd/amdgpu/../include/navi14_ip_offset.h:172:29: warning: 
‘UMC_BASE’ defined but not used [-Wunused-const-variable=]
 172 | static const struct IP_BASE UMC_BASE ={ { { { 0x00014000, 0x02425800, 0, 
0, 0 } },
 | ^~~~
 drivers/gpu/drm/amd/amdgpu/../include/navi14_ip_offset.h:151:29: warning: 
‘SDMA_BASE’ defined but not used [-Wunused-const-variable=]
 151 | static const struct IP_BASE SDMA_BASE ={ { { { 0x1260, 0xA000, 
0x02402C00, 0, 0 } },
 | ^

NB: Snipped a few of these

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/amd/include/navi14_ip_offset.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/include/navi14_ip_offset.h 
b/drivers/gpu/drm/amd/include/navi14_ip_offset.h
index ecdd9eabe0dc8..c39ef651adc6f 100644
--- a/drivers/gpu/drm/amd/include/navi14_ip_offset.h
+++ b/drivers/gpu/drm/amd/include/navi14_ip_offset.h
@@ -33,7 +33,7 @@ struct IP_BASE_INSTANCE
 struct IP_BASE
 {
 struct IP_BASE_INSTANCE instance[MAX_INSTANCE];
-};
+} __maybe_unused;
 
 
 static const struct IP_BASE ATHUB_BASE ={ { { { 0x0C00, 0x02408C00, 0, 0, 
0 } },
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 30/40] drm/amd/include/dimgrey_cavefish_ip_offset: Mark top-level IP_BASE as __maybe_unused

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

In file included from drivers/gpu/drm/amd/amdgpu/dimgrey_cavefish_reg_init.c:28:
drivers/gpu/drm/amd/amdgpu/../include/dimgrey_cavefish_ip_offset.h:151:29: 
warning: ‘UMC_BASE’ defined but not used [-Wunused-const-variable=]
151 | static const struct IP_BASE UMC_BASE = { { { { 0x00014000, 0x02425800, 0, 
0, 0, 0 } },
| ^~~~
drivers/gpu/drm/amd/amdgpu/../include/dimgrey_cavefish_ip_offset.h:81:29: 
warning: ‘FUSE_BASE’ defined but not used [-Wunused-const-variable=]
81 | static const struct IP_BASE FUSE_BASE = { { { { 0x00017400, 0x02401400, 0, 
0, 0, 0 } },
| ^
drivers/gpu/drm/amd/amdgpu/../include/dimgrey_cavefish_ip_offset.h:74:29: 
warning: ‘DPCS_BASE’ defined but not used [-Wunused-const-variable=]
74 | static const struct IP_BASE DPCS_BASE = { { { { 0x0012, 0x00C0, 
0x34C0, 0x9000, 0x02403C00, 0 } },
| ^

NB: Snipped lots of these

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Tao Zhou 
Cc: Hawking Zhang 
Cc: Jiansong Chen 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/amd/include/dimgrey_cavefish_ip_offset.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/include/dimgrey_cavefish_ip_offset.h 
b/drivers/gpu/drm/amd/include/dimgrey_cavefish_ip_offset.h
index b41263de8a9b6..f84996a73de94 100644
--- a/drivers/gpu/drm/amd/include/dimgrey_cavefish_ip_offset.h
+++ b/drivers/gpu/drm/amd/include/dimgrey_cavefish_ip_offset.h
@@ -33,7 +33,7 @@ struct IP_BASE_INSTANCE
 struct IP_BASE
 {
 struct IP_BASE_INSTANCE instance[MAX_INSTANCE];
-};
+} __maybe_unused;
 
 
 static const struct IP_BASE ATHUB_BASE = { { { { 0x0C00, 0x02408C00, 0, 0, 
0, 0 } },
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 39/40] drm/amd/amdgpu/gmc_v9_0: Remove unused table 'ecc_umc_mcumc_status_addrs'

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c:382:23: warning: 
‘ecc_umc_mcumc_status_addrs’ defined but not used [-Wunused-const-variable=]

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 35 ---
 1 file changed, 35 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 0c3421d587e87..fbee43b4ba64d 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -379,41 +379,6 @@ static const uint32_t ecc_umc_mcumc_ctrl_mask_addrs[] = {
(0x001d43e0 + 0x1800),
 };
 
-static const uint32_t ecc_umc_mcumc_status_addrs[] = {
-   (0x000143c2 + 0x),
-   (0x000143c2 + 0x0800),
-   (0x000143c2 + 0x1000),
-   (0x000143c2 + 0x1800),
-   (0x000543c2 + 0x),
-   (0x000543c2 + 0x0800),
-   (0x000543c2 + 0x1000),
-   (0x000543c2 + 0x1800),
-   (0x000943c2 + 0x),
-   (0x000943c2 + 0x0800),
-   (0x000943c2 + 0x1000),
-   (0x000943c2 + 0x1800),
-   (0x000d43c2 + 0x),
-   (0x000d43c2 + 0x0800),
-   (0x000d43c2 + 0x1000),
-   (0x000d43c2 + 0x1800),
-   (0x001143c2 + 0x),
-   (0x001143c2 + 0x0800),
-   (0x001143c2 + 0x1000),
-   (0x001143c2 + 0x1800),
-   (0x001543c2 + 0x),
-   (0x001543c2 + 0x0800),
-   (0x001543c2 + 0x1000),
-   (0x001543c2 + 0x1800),
-   (0x001943c2 + 0x),
-   (0x001943c2 + 0x0800),
-   (0x001943c2 + 0x1000),
-   (0x001943c2 + 0x1800),
-   (0x001d43c2 + 0x),
-   (0x001d43c2 + 0x0800),
-   (0x001d43c2 + 0x1000),
-   (0x001d43c2 + 0x1800),
-};
-
 static int gmc_v9_0_ecc_interrupt_state(struct amdgpu_device *adev,
struct amdgpu_irq_src *src,
unsigned type,
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 16/40] drm/amd/amdgpu/si_dma: Fix a bunch of function documentation issues

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/amd/amdgpu/si_dma.c:92: warning: Function parameter or member 
'addr' not described in 'si_dma_ring_emit_fence'
 drivers/gpu/drm/amd/amdgpu/si_dma.c:92: warning: Function parameter or member 
'seq' not described in 'si_dma_ring_emit_fence'
 drivers/gpu/drm/amd/amdgpu/si_dma.c:92: warning: Function parameter or member 
'flags' not described in 'si_dma_ring_emit_fence'
 drivers/gpu/drm/amd/amdgpu/si_dma.c:92: warning: Excess function parameter 
'fence' description in 'si_dma_ring_emit_fence'
 drivers/gpu/drm/amd/amdgpu/si_dma.c:252: warning: Function parameter or member 
'timeout' not described in 'si_dma_ring_test_ib'
 drivers/gpu/drm/amd/amdgpu/si_dma.c:408: warning: Function parameter or member 
'ring' not described in 'si_dma_ring_pad_ib'
 drivers/gpu/drm/amd/amdgpu/si_dma.c:446: warning: Function parameter or member 
'vmid' not described in 'si_dma_ring_emit_vm_flush'
 drivers/gpu/drm/amd/amdgpu/si_dma.c:446: warning: Function parameter or member 
'pd_addr' not described in 'si_dma_ring_emit_vm_flush'
 drivers/gpu/drm/amd/amdgpu/si_dma.c:446: warning: Excess function parameter 
'vm' description in 'si_dma_ring_emit_vm_flush'
 drivers/gpu/drm/amd/amdgpu/si_dma.c:781: warning: Function parameter or member 
'ib' not described in 'si_dma_emit_copy_buffer'
 drivers/gpu/drm/amd/amdgpu/si_dma.c:781: warning: Function parameter or member 
'tmz' not described in 'si_dma_emit_copy_buffer'
 drivers/gpu/drm/amd/amdgpu/si_dma.c:781: warning: Excess function parameter 
'ring' description in 'si_dma_emit_copy_buffer'
 drivers/gpu/drm/amd/amdgpu/si_dma.c:804: warning: Function parameter or member 
'ib' not described in 'si_dma_emit_fill_buffer'
 drivers/gpu/drm/amd/amdgpu/si_dma.c:804: warning: Excess function parameter 
'ring' description in 'si_dma_emit_fill_buffer'

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Sumit Semwal 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Cc: linux-me...@vger.kernel.org
Cc: linaro-mm-...@lists.linaro.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/amd/amdgpu/si_dma.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/si_dma.c 
b/drivers/gpu/drm/amd/amdgpu/si_dma.c
index 7d2bbcbe547b2..540dced190f33 100644
--- a/drivers/gpu/drm/amd/amdgpu/si_dma.c
+++ b/drivers/gpu/drm/amd/amdgpu/si_dma.c
@@ -81,7 +81,9 @@ static void si_dma_ring_emit_ib(struct amdgpu_ring *ring,
  * si_dma_ring_emit_fence - emit a fence on the DMA ring
  *
  * @ring: amdgpu ring pointer
- * @fence: amdgpu fence object
+ * @addr: address
+ * @seq: sequence number
+ * @flags: fence related flags
  *
  * Add a DMA fence packet to the ring to write
  * the fence seq number and DMA trap packet to generate
@@ -244,6 +246,7 @@ static int si_dma_ring_test_ring(struct amdgpu_ring *ring)
  * si_dma_ring_test_ib - test an IB on the DMA engine
  *
  * @ring: amdgpu_ring structure holding ring information
+ * @timeout: timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
  *
  * Test a simple IB in the DMA ring (VI).
  * Returns 0 on success, error on failure.
@@ -401,6 +404,7 @@ static void si_dma_vm_set_pte_pde(struct amdgpu_ib *ib,
 /**
  * si_dma_pad_ib - pad the IB to the required number of dw
  *
+ * @ring: amdgpu_ring pointer
  * @ib: indirect buffer to fill with padding
  *
  */
@@ -436,7 +440,8 @@ static void si_dma_ring_emit_pipeline_sync(struct 
amdgpu_ring *ring)
  * si_dma_ring_emit_vm_flush - cik vm flush using sDMA
  *
  * @ring: amdgpu_ring pointer
- * @vm: amdgpu_vm pointer
+ * @vmid: vmid number to use
+ * @pd_addr: address
  *
  * Update the page table base and flush the VM TLB
  * using sDMA (VI).
@@ -764,10 +769,11 @@ static void si_dma_set_irq_funcs(struct amdgpu_device 
*adev)
 /**
  * si_dma_emit_copy_buffer - copy buffer using the sDMA engine
  *
- * @ring: amdgpu_ring structure holding ring information
+ * @ib: indirect buffer to copy to
  * @src_offset: src GPU address
  * @dst_offset: dst GPU address
  * @byte_count: number of bytes to xfer
+ * @tmz: unused
  *
  * Copy GPU buffers using the DMA engine (VI).
  * Used by the amdgpu ttm implementation to move pages if
@@ -790,7 +796,7 @@ static void si_dma_emit_copy_buffer(struct amdgpu_ib *ib,
 /**
  * si_dma_emit_fill_buffer - fill buffer using the sDMA engine
  *
- * @ring: amdgpu_ring structure holding ring information
+ * @ib: indirect buffer to copy to
  * @src_data: value to write to buffer
  * @dst_offset: dst GPU address
  * @byte_count: number of bytes to xfer
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 21/40] drm/amd/amdgpu/uvd_v3_1: Fix-up some documentation issues

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c:91: warning: Function parameter or 
member 'job' not described in 'uvd_v3_1_ring_emit_ib'
 drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c:91: warning: Function parameter or 
member 'flags' not described in 'uvd_v3_1_ring_emit_ib'
 drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c:108: warning: Function parameter or 
member 'addr' not described in 'uvd_v3_1_ring_emit_fence'
 drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c:108: warning: Function parameter or 
member 'seq' not described in 'uvd_v3_1_ring_emit_fence'
 drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c:108: warning: Function parameter or 
member 'flags' not described in 'uvd_v3_1_ring_emit_fence'
 drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c:108: warning: Excess function parameter 
'fence' description in 'uvd_v3_1_ring_emit_fence'
 drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c:625: warning: Function parameter or 
member 'handle' not described in 'uvd_v3_1_hw_init'
 drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c:625: warning: Excess function parameter 
'adev' description in 'uvd_v3_1_hw_init'
 drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c:692: warning: Function parameter or 
member 'handle' not described in 'uvd_v3_1_hw_fini'
 drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c:692: warning: Excess function parameter 
'adev' description in 'uvd_v3_1_hw_fini'

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Sonny Jiang 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c 
b/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c
index 7cf4b11a65c5c..143ba7a41f41f 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c
@@ -80,7 +80,9 @@ static void uvd_v3_1_ring_set_wptr(struct amdgpu_ring *ring)
  * uvd_v3_1_ring_emit_ib - execute indirect buffer
  *
  * @ring: amdgpu_ring pointer
+ * @job: unused
  * @ib: indirect buffer to execute
+ * @flags: unused
  *
  * Write ring commands to execute the indirect buffer
  */
@@ -99,7 +101,9 @@ static void uvd_v3_1_ring_emit_ib(struct amdgpu_ring *ring,
  * uvd_v3_1_ring_emit_fence - emit an fence & trap command
  *
  * @ring: amdgpu_ring pointer
- * @fence: fence to emit
+ * @addr: address
+ * @seq: sequence number
+ * @flags: fence related flags
  *
  * Write a fence and a trap command to the ring.
  */
@@ -617,7 +621,7 @@ static void uvd_v3_1_enable_mgcg(struct amdgpu_device *adev,
 /**
  * uvd_v3_1_hw_init - start and test UVD block
  *
- * @adev: amdgpu_device pointer
+ * @handle: handle used to pass amdgpu_device pointer
  *
  * Initialize the hardware, boot up the VCPU and do some testing
  */
@@ -684,7 +688,7 @@ static int uvd_v3_1_hw_init(void *handle)
 /**
  * uvd_v3_1_hw_fini - stop the hardware block
  *
- * @adev: amdgpu_device pointer
+ * @handle: handle used to pass amdgpu_device pointer
  *
  * Stop the UVD block, mark ring as not ready any more
  */
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 09/40] drm/amd/amdgpu/gfx_v7_0: Clean-up a bunch of kernel-doc related issues

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:1590: warning: Function parameter or 
member 'instance' not described in 'gfx_v7_0_select_se_sh'
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:1788: warning: Excess function parameter 
'se_num' description in 'gfx_v7_0_setup_rb'
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:1788: warning: Excess function parameter 
'sh_per_se' description in 'gfx_v7_0_setup_rb'
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:1852: warning: Excess function parameter 
'adev' description in 'DEFAULT_SH_MEM_BASES'
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:2086: warning: Excess function parameter 
'adev' description in 'gfx_v7_0_ring_test_ring'
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:2130: warning: Function parameter or 
member 'ring' not described in 'gfx_v7_0_ring_emit_hdp_flush'
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:2130: warning: Excess function parameter 
'adev' description in 'gfx_v7_0_ring_emit_hdp_flush'
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:2130: warning: Excess function parameter 
'ridx' description in 'gfx_v7_0_ring_emit_hdp_flush'
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:2182: warning: Function parameter or 
member 'ring' not described in 'gfx_v7_0_ring_emit_fence_gfx'
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:2182: warning: Function parameter or 
member 'addr' not described in 'gfx_v7_0_ring_emit_fence_gfx'
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:2182: warning: Function parameter or 
member 'seq' not described in 'gfx_v7_0_ring_emit_fence_gfx'
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:2182: warning: Function parameter or 
member 'flags' not described in 'gfx_v7_0_ring_emit_fence_gfx'
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:2182: warning: Excess function parameter 
'adev' description in 'gfx_v7_0_ring_emit_fence_gfx'
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:2182: warning: Excess function parameter 
'fence' description in 'gfx_v7_0_ring_emit_fence_gfx'
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:2224: warning: Function parameter or 
member 'ring' not described in 'gfx_v7_0_ring_emit_fence_compute'
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:2224: warning: Function parameter or 
member 'addr' not described in 'gfx_v7_0_ring_emit_fence_compute'
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:2224: warning: Function parameter or 
member 'seq' not described in 'gfx_v7_0_ring_emit_fence_compute'
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:2224: warning: Function parameter or 
member 'flags' not described in 'gfx_v7_0_ring_emit_fence_compute'
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:2224: warning: Excess function parameter 
'adev' description in 'gfx_v7_0_ring_emit_fence_compute'
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:2224: warning: Excess function parameter 
'fence' description in 'gfx_v7_0_ring_emit_fence_compute'
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:2260: warning: Function parameter or 
member 'job' not described in 'gfx_v7_0_ring_emit_ib_gfx'
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:2260: warning: Function parameter or 
member 'flags' not described in 'gfx_v7_0_ring_emit_ib_gfx'
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:2351: warning: Function parameter or 
member 'timeout' not described in 'gfx_v7_0_ring_test_ib'
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:3244: warning: Function parameter or 
member 'ring' not described in 'gfx_v7_0_ring_emit_vm_flush'
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:3244: warning: Function parameter or 
member 'vmid' not described in 'gfx_v7_0_ring_emit_vm_flush'
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:3244: warning: Function parameter or 
member 'pd_addr' not described in 'gfx_v7_0_ring_emit_vm_flush'
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:3244: warning: Excess function parameter 
'adev' description in 'gfx_v7_0_ring_emit_vm_flush'

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Sumit Semwal 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Cc: linux-me...@vger.kernel.org
Cc: linaro-mm-...@lists.linaro.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 33 +++
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index 04e1e92f5f3cf..f2490f915a8be 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -1580,10 +1580,10 @@ static void gfx_v7_0_tiling_mode_table_init(struct 
amdgpu_device *adev)
  * @adev: amdgpu_device pointer
  * @se_num: shader engine to address
  * @sh_num: sh block to address
+ * @instance: Certain registers are instanced per SE or SH.
+ *0x means broadcast to all SEs or SHs (CIK).
  *
- * Select which SE, SH combinations to address. Certain
- * registers are instanced per SE or SH.  0x means
- * broadcast to all SEs or SHs (CIK).
+ * Select which SE, SH combinations to address.
  */
 static void gfx_v7_0_select_se_sh(struct amdgpu_device *adev,
  u32 se_num, u32 

[PATCH 28/40] drm/amd/include/sienna_cichlid_ip_offset: Mark top-level IP_BASE as __maybe_unused

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 In file included from drivers/gpu/drm/amd/amdgpu/sienna_cichlid_reg_init.c:28:
 drivers/gpu/drm/amd/amdgpu/../include/sienna_cichlid_ip_offset.h:186:29: 
warning: ‘USB0_BASE’ defined but not used [-Wunused-const-variable=]
 186 | static const struct IP_BASE USB0_BASE = { { { { 0x0242A800, 0x05B0, 
0, 0, 0 } },
 | ^
 drivers/gpu/drm/amd/amdgpu/../include/sienna_cichlid_ip_offset.h:179:29: 
warning: ‘UMC_BASE’ defined but not used [-Wunused-const-variable=]
 179 | static const struct IP_BASE UMC_BASE = { { { { 0x00014000, 0x02425800, 
0, 0, 0 } },
 | ^~~~
 drivers/gpu/drm/amd/amdgpu/../include/sienna_cichlid_ip_offset.h:158:29: 
warning: ‘SDMA1_BASE’ defined but not used [-Wunused-const-variable=]
 158 | static const struct IP_BASE SDMA1_BASE = { { { { 0x1260, 0xA000, 
0x0001C000, 0x02402C00, 0 } },
 | ^~

NB: Snipped lots of these

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Hawking Zhang 
Cc: Likun Gao 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/amd/include/sienna_cichlid_ip_offset.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/include/sienna_cichlid_ip_offset.h 
b/drivers/gpu/drm/amd/include/sienna_cichlid_ip_offset.h
index 06800c6fa0495..b07bc2dd895dc 100644
--- a/drivers/gpu/drm/amd/include/sienna_cichlid_ip_offset.h
+++ b/drivers/gpu/drm/amd/include/sienna_cichlid_ip_offset.h
@@ -33,7 +33,7 @@ struct IP_BASE_INSTANCE
 struct IP_BASE
 {
 struct IP_BASE_INSTANCE instance[MAX_INSTANCE];
-};
+} __maybe_unused;
 
 
 static const struct IP_BASE ATHUB_BASE = { { { { 0x0C00, 0x02408C00, 0, 0, 
0 } },
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 37/40] drm/amd/amdgpu/gmc_v8_0: Fix more issues attributed to copy/paste

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c:618: warning: Function parameter or 
member 'flush_type' not described in 'gmc_v8_0_flush_gpu_tlb_pasid'
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c:618: warning: Function parameter or 
member 'all_hub' not described in 'gmc_v8_0_flush_gpu_tlb_pasid'
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c:657: warning: Function parameter or 
member 'vmhub' not described in 'gmc_v8_0_flush_gpu_tlb'
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c:657: warning: Function parameter or 
member 'flush_type' not described in 'gmc_v8_0_flush_gpu_tlb'
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c:998: warning: Function parameter or 
member 'pasid' not described in 'gmc_v8_0_vm_decode_fault'

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
index 0f32a8002c3d7..41c1d8e812b88 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -609,6 +609,8 @@ static int gmc_v8_0_mc_init(struct amdgpu_device *adev)
  *
  * @adev: amdgpu_device pointer
  * @pasid: pasid to be flush
+ * @flush_type: unused
+ * @all_hub: unused
  *
  * Flush the TLB for the requested pasid.
  */
@@ -649,6 +651,8 @@ static int gmc_v8_0_flush_gpu_tlb_pasid(struct 
amdgpu_device *adev,
  *
  * @adev: amdgpu_device pointer
  * @vmid: vm instance to flush
+ * @vmhub: unused
+ * @flush_type: unused
  *
  * Flush the TLB for the requested page table (VI).
  */
@@ -990,6 +994,7 @@ static void gmc_v8_0_gart_disable(struct amdgpu_device 
*adev)
  * @status: VM_CONTEXT1_PROTECTION_FAULT_STATUS register value
  * @addr: VM_CONTEXT1_PROTECTION_FAULT_ADDR register value
  * @mc_client: VM_CONTEXT1_PROTECTION_FAULT_MCCLIENT register value
+ * @pasid: debug logging only - no functional use
  *
  * Print human readable fault information (VI).
  */
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 36/40] drm/amd/amdgpu/gmc_v7_0: Add some missing kernel-doc descriptions

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c:433: warning: Function parameter or 
member 'flush_type' not described in 'gmc_v7_0_flush_gpu_tlb_pasid'
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c:433: warning: Function parameter or 
member 'all_hub' not described in 'gmc_v7_0_flush_gpu_tlb_pasid'
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c:471: warning: Function parameter or 
member 'vmhub' not described in 'gmc_v7_0_flush_gpu_tlb'
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c:471: warning: Function parameter or 
member 'flush_type' not described in 'gmc_v7_0_flush_gpu_tlb'
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c:771: warning: Function parameter or 
member 'pasid' not described in 'gmc_v7_0_vm_decode_fault'

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
index 80c146df338aa..fe71c89ecd26f 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
@@ -424,6 +424,8 @@ static int gmc_v7_0_mc_init(struct amdgpu_device *adev)
  *
  * @adev: amdgpu_device pointer
  * @pasid: pasid to be flush
+ * @flush_type: unused
+ * @all_hub: unused
  *
  * Flush the TLB for the requested pasid.
  */
@@ -463,7 +465,9 @@ static int gmc_v7_0_flush_gpu_tlb_pasid(struct 
amdgpu_device *adev,
  *
  * @adev: amdgpu_device pointer
  * @vmid: vm instance to flush
- *
+ * @vmhub: unused
+ * @flush_type: unused
+ * *
  * Flush the TLB for the requested page table (CIK).
  */
 static void gmc_v7_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
@@ -763,6 +767,7 @@ static void gmc_v7_0_gart_disable(struct amdgpu_device 
*adev)
  * @status: VM_CONTEXT1_PROTECTION_FAULT_STATUS register value
  * @addr: VM_CONTEXT1_PROTECTION_FAULT_ADDR register value
  * @mc_client: VM_CONTEXT1_PROTECTION_FAULT_MCCLIENT register value
+ * @pasid: debug logging only - no functional use
  *
  * Print human readable fault information (CIK).
  */
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 33/40] drm/amd/amdgpu/cik_sdma: Add one and remove another function param description

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/amd/amdgpu/cik_sdma.c:282: warning: Function parameter or 
member 'flags' not described in 'cik_sdma_ring_emit_fence'
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c:282: warning: Excess function parameter 
'fence' description in 'cik_sdma_ring_emit_fence'

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Sumit Semwal 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Cc: linux-me...@vger.kernel.org
Cc: linaro-mm-...@lists.linaro.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c 
b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
index f1e9966e7244e..28a64de8ae0e6 100644
--- a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
@@ -271,7 +271,7 @@ static void cik_sdma_ring_emit_hdp_flush(struct amdgpu_ring 
*ring)
  * @ring: amdgpu ring pointer
  * @addr: address
  * @seq: sequence number
- * @fence: amdgpu fence object
+ * @flags: fence related flags
  *
  * Add a DMA fence packet to the ring to write
  * the fence seq number and DMA trap packet to generate
@@ -279,7 +279,7 @@ static void cik_sdma_ring_emit_hdp_flush(struct amdgpu_ring 
*ring)
  */
 static void cik_sdma_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64 
seq,
 unsigned flags)
-{
+  {
bool write64bit = flags & AMDGPU_FENCE_FLAG_64BIT;
/* write the fence */
amdgpu_ring_write(ring, SDMA_PACKET(SDMA_OPCODE_FENCE, 0, 0));
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amd/powerplay: fix spelling mistake "smu_state_memroy_block" -> "smu_state_memory_block"

[Colin King]

From: Colin Ian King 

The struct name smu_state_memroy_block contains a spelling mistake, rename
it to smu_state_memory_block

Fixes: 8554e67d6e22 ("drm/amd/powerplay: implement power_dpm_state sys 
interface for SMU11")
Signed-off-by: Colin Ian King 
---
 drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h 
b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
index 7550757cc059..a559ea2204c1 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
@@ -99,7 +99,7 @@ struct smu_state_display_block {
bool  enable_vari_bright;
 };
 
-struct smu_state_memroy_block {
+struct smu_state_memory_block {
bool  dll_off;
uint8_t m3arb;
uint8_t unused[3];
@@ -146,7 +146,7 @@ struct smu_power_state {
struct smu_state_validation_block validation;
struct smu_state_pcie_block   pcie;
struct smu_state_display_blockdisplay;
-   struct smu_state_memroy_block memory;
+   struct smu_state_memory_block memory;
struct smu_state_software_algorithm_block software;
struct smu_uvd_clocks uvd_clocks;
struct smu_hw_power_state hardware;
-- 
2.28.0

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 24/40] drm/amd/include/navi10_ip_offset: Mark top-level IP_BASE as __maybe_unused

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 In file included from drivers/gpu/drm/amd/amdgpu/navi10_reg_init.c:27:
 drivers/gpu/drm/amd/amdgpu/../include/navi10_ip_offset.h:127:29: warning: 
‘UMC_BASE’ defined but not used [-Wunused-const-variable=]
 127 | static const struct IP_BASE UMC_BASE ={ { { { 0x00014000, 0, 0, 0, 0, 0 
} },
 | ^~~~
 drivers/gpu/drm/amd/amdgpu/../include/navi10_ip_offset.h:109:29: warning: 
‘RSMU_BASE’ defined but not used [-Wunused-const-variable=]
 109 | static const struct IP_BASE RSMU_BASE = { { { { 0x00012000, 0, 0, 0, 0, 
0 } },
 | ^
 drivers/gpu/drm/amd/amdgpu/../include/navi10_ip_offset.h:61:29: warning: 
‘FUSE_BASE’ defined but not used [-Wunused-const-variable=]
 61 | static const struct IP_BASE FUSE_BASE ={ { { { 0x00017400, 0, 0, 0, 0, 0 
} },
 | ^

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/amd/include/navi10_ip_offset.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/include/navi10_ip_offset.h 
b/drivers/gpu/drm/amd/include/navi10_ip_offset.h
index d4a9ddc7782ff..d6824bb6139db 100644
--- a/drivers/gpu/drm/amd/include/navi10_ip_offset.h
+++ b/drivers/gpu/drm/amd/include/navi10_ip_offset.h
@@ -31,7 +31,7 @@ struct IP_BASE_INSTANCE {
  
 struct IP_BASE {
struct IP_BASE_INSTANCE instance[MAX_INSTANCE];
-};
+} __maybe_unused;
 
 
 static const struct IP_BASE ATHUB_BASE={ { { { 0x0C00, 0, 0, 
0, 0, 0 } },
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 25/40] drm/amd/include/arct_ip_offset: Mark top-level IP_BASE definition as __maybe_unused

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 In file included from drivers/gpu/drm/amd/amdgpu/arct_reg_init.c:27:
 drivers/gpu/drm/amd/amdgpu/../include/arct_ip_offset.h:227:29: warning: 
‘DBGU_IO_BASE’ defined but not used [-Wunused-const-variable=]
 227 | static const struct IP_BASE DBGU_IO_BASE ={ { { { 0x01E0, 
0x000125A0, 0x0040B400, 0, 0, 0 } },
 | ^~~~
 drivers/gpu/drm/amd/amdgpu/../include/arct_ip_offset.h:127:29: warning: 
‘PCIE0_BASE’ defined but not used [-Wunused-const-variable=]
 127 | static const struct IP_BASE PCIE0_BASE ={ { { { 0x000128C0, 0x00411800, 
0x0444, 0, 0, 0 } },
 | ^~
 In file included from drivers/gpu/drm/amd/amdgpu/arct_reg_init.c:27:
 drivers/gpu/drm/amd/amdgpu/../include/arct_ip_offset.h:63:29: warning: 
‘FUSE_BASE’ defined but not used [-Wunused-const-variable=]
 63 | static const struct IP_BASE FUSE_BASE ={ { { { 0x000120A0, 0x00017400, 
0x00401400, 0, 0, 0 } },
 | ^

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/amd/include/arct_ip_offset.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/include/arct_ip_offset.h 
b/drivers/gpu/drm/amd/include/arct_ip_offset.h
index a7791a9e1f905..af1c46991429b 100644
--- a/drivers/gpu/drm/amd/include/arct_ip_offset.h
+++ b/drivers/gpu/drm/amd/include/arct_ip_offset.h
@@ -28,12 +28,12 @@
 struct IP_BASE_INSTANCE
 {
 unsigned int segment[MAX_SEGMENT];
-};
+} __maybe_unused;
 
 struct IP_BASE
 {
 struct IP_BASE_INSTANCE instance[MAX_INSTANCE];
-};
+} __maybe_unused;
 
 
 static const struct IP_BASE ATHUB_BASE={ { { { 0x0C20, 
0x00012460, 0x00408C00, 0, 0, 0 } },
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 06/40] drm/amd/amdgpu/uvd_v4_2: Fix some kernel-doc misdemeanours

[Lee Jones]

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c:157: warning: Function parameter or 
member 'handle' not described in 'uvd_v4_2_hw_init'
 drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c:157: warning: Excess function parameter 
'adev' description in 'uvd_v4_2_hw_init'
 drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c:212: warning: Function parameter or 
member 'handle' not described in 'uvd_v4_2_hw_fini'
 drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c:212: warning: Excess function parameter 
'adev' description in 'uvd_v4_2_hw_fini'
 drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c:446: warning: Function parameter or 
member 'addr' not described in 'uvd_v4_2_ring_emit_fence'
 drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c:446: warning: Function parameter or 
member 'seq' not described in 'uvd_v4_2_ring_emit_fence'
 drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c:446: warning: Function parameter or 
member 'flags' not described in 'uvd_v4_2_ring_emit_fence'
 drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c:446: warning: Excess function parameter 
'fence' description in 'uvd_v4_2_ring_emit_fence'
 drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c:513: warning: Function parameter or 
member 'job' not described in 'uvd_v4_2_ring_emit_ib'
 drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c:513: warning: Function parameter or 
member 'flags' not described in 'uvd_v4_2_ring_emit_ib'

Cc: Alex Deucher 
Cc: "Christian König" 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: Lee Jones 
---
 drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c 
b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
index b0c0c438fc93c..2c8c35c3bca52 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c
@@ -149,7 +149,7 @@ static void uvd_v4_2_enable_mgcg(struct amdgpu_device *adev,
 /**
  * uvd_v4_2_hw_init - start and test UVD block
  *
- * @adev: amdgpu_device pointer
+ * @handle: handle used to pass amdgpu_device pointer
  *
  * Initialize the hardware, boot up the VCPU and do some testing
  */
@@ -204,7 +204,7 @@ static int uvd_v4_2_hw_init(void *handle)
 /**
  * uvd_v4_2_hw_fini - stop the hardware block
  *
- * @adev: amdgpu_device pointer
+ * @handle: handle used to pass amdgpu_device pointer
  *
  * Stop the UVD block, mark ring as not ready any more
  */
@@ -437,6 +437,8 @@ static void uvd_v4_2_stop(struct amdgpu_device *adev)
  * uvd_v4_2_ring_emit_fence - emit an fence & trap command
  *
  * @ring: amdgpu_ring pointer
+ * @addr: address
+ * @seq: sequence number
  * @fence: fence to emit
  *
  * Write a fence and a trap command to the ring.
@@ -502,7 +504,9 @@ static int uvd_v4_2_ring_test_ring(struct amdgpu_ring *ring)
  * uvd_v4_2_ring_emit_ib - execute indirect buffer
  *
  * @ring: amdgpu_ring pointer
+ * @job: unused
  * @ib: indirect buffer to execute
+ * @flags: unused
  *
  * Write ring commands to execute the indirect buffer
  */
-- 
2.25.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 1/1] drm/amdgpu: set default value of noretry to 1 for specified asic

[Zhang, Hawking]

[AMD Public Use]

Reviewed-by: Hawking Zhang 

Regards,
Hawking

-Original Message-
From: amd-gfx  On Behalf Of Stanley.Yang
Sent: Monday, November 23, 2020 21:14
To: amd-gfx@lists.freedesktop.org
Cc: Yang, Stanley 
Subject: [PATCH 1/1] drm/amdgpu: set default value of noretry to 1 for 
specified asic

noretry = 0 casue KFDGraphicsInterop test failed on SRIOV platform for vega10, 
so set noretry to 1 for vega10.

Signed-off-by: Stanley.Yang 
Change-Id: I241da5c20970ea889909997ff044d6e61642da81
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index a3d4325718d8..7bb544224540 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -425,6 +425,7 @@ void amdgpu_gmc_noretry_set(struct amdgpu_device *adev)
struct amdgpu_gmc *gmc = >gmc;
 
switch (adev->asic_type) {
+   case CHIP_VEGA10:
case CHIP_VEGA20:
case CHIP_NAVI10:
case CHIP_NAVI14:
--
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=04%7C01%7Chawking.zhang%40amd.com%7C6c424996a1ec4400854a08d88fb19a4b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637417340270280004%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=ZLKDjTTy2LGV936z6vnq93hv0GJ2EgsG1kDF70InMrM%3Dreserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 1/1] drm/amdgpu: only skip smc sdma sos ta and asd fw in SRIOV for navi12

[Zhang, Hawking]

[AMD Public Use]

@@ -593,7 +593,7 @@ static int sdma_v4_0_init_microcode(struct amdgpu_device 
*adev)
struct amdgpu_firmware_info *info = NULL;
const struct common_firmware_header *header = NULL;
 
-   if (amdgpu_sriov_vf(adev))
+   if (amdgpu_sriov_vf(adev) && (adev->asic_type == CHIP_NAVI12))

Navi12 doesn't integrate sdma v4. Why we need to check Navi12 in sdma v4 
function.

Regards,
Hawking

-Original Message-
From: amd-gfx  On Behalf Of Stanley.Yang
Sent: Monday, November 23, 2020 21:19
To: amd-gfx@lists.freedesktop.org; Chen, JingWen 
Cc: Yang, Stanley 
Subject: [PATCH 1/1] drm/amdgpu: only skip smc sdma sos ta and asd fw in SRIOV 
for navi12

The KFDTopologyTest.BasicTest will failed if skip smc, sdma, sos, ta and asd fw 
in SRIOV for vega10, so adjust above fw and skip load them in SRIOV only for 
navi12.

Signed-off-by: Stanley.Yang 
Change-Id: Id354be93723d7b5d769d73dc67c596af300305af
---
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c  | 2 +-
 drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c  | 2 +-
 drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c  | 2 +-
 drivers/gpu/drm/amd/pm/powerplay/smumgr/vega10_smumgr.c | 3 ++-
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c   | 2 +-
 5 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index 16b551f330a4..7e2f063120d8 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -593,7 +593,7 @@ static int sdma_v4_0_init_microcode(struct amdgpu_device 
*adev)
struct amdgpu_firmware_info *info = NULL;
const struct common_firmware_header *header = NULL;
 
-   if (amdgpu_sriov_vf(adev))
+   if (amdgpu_sriov_vf(adev) && (adev->asic_type == CHIP_NAVI12))
return 0;
 
DRM_DEBUG("\n");
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
index 9c72b95b7463..fad1cc394219 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
@@ -203,7 +203,7 @@ static int sdma_v5_0_init_microcode(struct amdgpu_device 
*adev)
const struct common_firmware_header *header = NULL;
const struct sdma_firmware_header_v1_0 *hdr;
 
-   if (amdgpu_sriov_vf(adev))
+   if (amdgpu_sriov_vf(adev) && (adev->asic_type == CHIP_NAVI12))
return 0;
 
DRM_DEBUG("\n");
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
index cb5a6f1437f8..674bc88c3ec1 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
@@ -153,7 +153,7 @@ static int sdma_v5_2_init_microcode(struct amdgpu_device 
*adev)
struct amdgpu_firmware_info *info = NULL;
const struct common_firmware_header *header = NULL;
 
-   if (amdgpu_sriov_vf(adev))
+   if (amdgpu_sriov_vf(adev) && (adev->asic_type == CHIP_NAVI12))
return 0;
 
DRM_DEBUG("\n");
diff --git a/drivers/gpu/drm/amd/pm/powerplay/smumgr/vega10_smumgr.c 
b/drivers/gpu/drm/amd/pm/powerplay/smumgr/vega10_smumgr.c
index daf122f24f23..192149e94f6c 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/smumgr/vega10_smumgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/smumgr/vega10_smumgr.c
@@ -208,8 +208,9 @@ static int vega10_smu_init(struct pp_hwmgr *hwmgr)
unsigned long tools_size;
int ret;
struct cgs_firmware_info info = {0};
+   struct amdgpu_device *adev = hwmgr->adev;
 
-   if (!amdgpu_sriov_vf((struct amdgpu_device *)hwmgr->adev)) {
+   if (!amdgpu_sriov_vf(adev) || (adev->asic_type != CHIP_NAVI12)) {
ret = cgs_get_firmware_info(hwmgr->device,
CGS_UCODE_ID_SMU,
);
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c 
b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 1904df5a3e20..80c0bfaed097 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -847,7 +847,7 @@ static int smu_sw_init(void *handle)
smu->smu_dpm.dpm_level = AMD_DPM_FORCED_LEVEL_AUTO;
smu->smu_dpm.requested_dpm_level = AMD_DPM_FORCED_LEVEL_AUTO;
 
-   if (!amdgpu_sriov_vf(adev)) {
+   if (!amdgpu_sriov_vf(adev) || (adev->asic_type != CHIP_NAVI12)) {
ret = smu_init_microcode(smu);
if (ret) {
dev_err(adev->dev, "Failed to load smu firmware!\n");
--
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org

[PATCH 1/1] drm/amdgpu: only skip smc sdma sos ta and asd fw in SRIOV for navi12

[Stanley . Yang]

The KFDTopologyTest.BasicTest will failed if skip smc, sdma, sos, ta
and asd fw in SRIOV for vega10, so adjust above fw and skip load them
in SRIOV only for navi12.

Signed-off-by: Stanley.Yang 
Change-Id: Id354be93723d7b5d769d73dc67c596af300305af
---
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c  | 2 +-
 drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c  | 2 +-
 drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c  | 2 +-
 drivers/gpu/drm/amd/pm/powerplay/smumgr/vega10_smumgr.c | 3 ++-
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c   | 2 +-
 5 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index 16b551f330a4..7e2f063120d8 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -593,7 +593,7 @@ static int sdma_v4_0_init_microcode(struct amdgpu_device 
*adev)
struct amdgpu_firmware_info *info = NULL;
const struct common_firmware_header *header = NULL;
 
-   if (amdgpu_sriov_vf(adev))
+   if (amdgpu_sriov_vf(adev) && (adev->asic_type == CHIP_NAVI12))
return 0;
 
DRM_DEBUG("\n");
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
index 9c72b95b7463..fad1cc394219 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
@@ -203,7 +203,7 @@ static int sdma_v5_0_init_microcode(struct amdgpu_device 
*adev)
const struct common_firmware_header *header = NULL;
const struct sdma_firmware_header_v1_0 *hdr;
 
-   if (amdgpu_sriov_vf(adev))
+   if (amdgpu_sriov_vf(adev) && (adev->asic_type == CHIP_NAVI12))
return 0;
 
DRM_DEBUG("\n");
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
index cb5a6f1437f8..674bc88c3ec1 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
@@ -153,7 +153,7 @@ static int sdma_v5_2_init_microcode(struct amdgpu_device 
*adev)
struct amdgpu_firmware_info *info = NULL;
const struct common_firmware_header *header = NULL;
 
-   if (amdgpu_sriov_vf(adev))
+   if (amdgpu_sriov_vf(adev) && (adev->asic_type == CHIP_NAVI12))
return 0;
 
DRM_DEBUG("\n");
diff --git a/drivers/gpu/drm/amd/pm/powerplay/smumgr/vega10_smumgr.c 
b/drivers/gpu/drm/amd/pm/powerplay/smumgr/vega10_smumgr.c
index daf122f24f23..192149e94f6c 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/smumgr/vega10_smumgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/smumgr/vega10_smumgr.c
@@ -208,8 +208,9 @@ static int vega10_smu_init(struct pp_hwmgr *hwmgr)
unsigned long tools_size;
int ret;
struct cgs_firmware_info info = {0};
+   struct amdgpu_device *adev = hwmgr->adev;
 
-   if (!amdgpu_sriov_vf((struct amdgpu_device *)hwmgr->adev)) {
+   if (!amdgpu_sriov_vf(adev) || (adev->asic_type != CHIP_NAVI12)) {
ret = cgs_get_firmware_info(hwmgr->device,
CGS_UCODE_ID_SMU,
);
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c 
b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 1904df5a3e20..80c0bfaed097 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -847,7 +847,7 @@ static int smu_sw_init(void *handle)
smu->smu_dpm.dpm_level = AMD_DPM_FORCED_LEVEL_AUTO;
smu->smu_dpm.requested_dpm_level = AMD_DPM_FORCED_LEVEL_AUTO;
 
-   if (!amdgpu_sriov_vf(adev)) {
+   if (!amdgpu_sriov_vf(adev) || (adev->asic_type != CHIP_NAVI12)) {
ret = smu_init_microcode(smu);
if (ret) {
dev_err(adev->dev, "Failed to load smu firmware!\n");
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 1/1] drm/amdgpu: set default value of noretry to 1 for specified asic

[Stanley . Yang]

noretry = 0 casue KFDGraphicsInterop test failed on SRIOV platform
for vega10, so set noretry to 1 for vega10.

Signed-off-by: Stanley.Yang 
Change-Id: I241da5c20970ea889909997ff044d6e61642da81
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index a3d4325718d8..7bb544224540 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -425,6 +425,7 @@ void amdgpu_gmc_noretry_set(struct amdgpu_device *adev)
struct amdgpu_gmc *gmc = >gmc;
 
switch (adev->asic_type) {
+   case CHIP_VEGA10:
case CHIP_VEGA20:
case CHIP_NAVI10:
case CHIP_NAVI14:
-- 
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang

[Gustavo A. R. Silva]

On Sun, Nov 22, 2020 at 11:53:55AM -0800, James Bottomley wrote:
> On Sun, 2020-11-22 at 11:22 -0800, Joe Perches wrote:
> > On Sun, 2020-11-22 at 11:12 -0800, James Bottomley wrote:
> > > On Sun, 2020-11-22 at 10:25 -0800, Joe Perches wrote:
> > > > On Sun, 2020-11-22 at 10:21 -0800, James Bottomley wrote:
> > > > > Please tell me our reward for all this effort isn't a single
> > > > > missing error print.
> > > > 
> > > > There were quite literally dozens of logical defects found
> > > > by the fallthrough additions.  Very few were logging only.
> > > 
> > > So can you give us the best examples (or indeed all of them if
> > > someone is keeping score)?  hopefully this isn't a US election
> > > situation ...
> > 
> > Gustavo?  Are you running for congress now?
> > 
> > https://lwn.net/Articles/794944/
> 
> That's 21 reported fixes of which about 50% seem to produce no change
> in code behaviour at all, a quarter seem to have no user visible effect
> with the remaining quarter producing unexpected errors on obscure
> configuration parameters, which is why no-one really noticed them
> before.

The really important point here is the number of bugs this has prevented
and will prevent in the future. See an example of this, below:

https://lore.kernel.org/linux-iio/20190813135802.gb27...@kroah.com/

This work is still relevant, even if the total number of issues/bugs
we find in the process is zero (which is not the case).

"The sucky thing about doing hard work to deploy hardening is that the
result is totally invisible by definition (things not happening) [..]"
- Dmitry Vyukov

Thanks
--
Gustavo





___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: unpack dma_fence_chain containers during sync

[Christian König]

Ping, Pierre/Marek does this change works as expected?

Regards,
Christian.

Am 18.11.20 um 14:20 schrieb Christian König:

This allows for optimizing the CPU round trip away.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c   |  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 27 
  drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h |  1 +
  3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 79342976fa76..68f9a4adf5d2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1014,7 +1014,7 @@ static int amdgpu_syncobj_lookup_and_add_to_sync(struct 
amdgpu_cs_parser *p,
return r;
}
  
-	r = amdgpu_sync_fence(>job->sync, fence);

+   r = amdgpu_sync_fence_chain(>job->sync, fence);
dma_fence_put(fence);
  
  	return r;

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
index 8ea6c49529e7..d0d64af06f54 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
@@ -28,6 +28,8 @@
   *Christian König 
   */
  
+#include 

+
  #include "amdgpu.h"
  #include "amdgpu_trace.h"
  #include "amdgpu_amdkfd.h"
@@ -169,6 +171,31 @@ int amdgpu_sync_fence(struct amdgpu_sync *sync, struct 
dma_fence *f)
return 0;
  }
  
+/**

+ * amdgpu_sync_fence_chain - unpack dma_fence_chain and sync
+ *
+ * @sync: sync object to add fence to
+ * @f: potential dma_fence_chain to sync to.
+ *
+ * Add the fences inside the chain to the sync object.
+ */
+int amdgpu_sync_fence_chain(struct amdgpu_sync *sync, struct dma_fence *f)
+{
+   int r;
+
+   dma_fence_chain_for_each(f, f) {
+   if (dma_fence_is_signaled(f))
+   continue;
+
+   r = amdgpu_sync_fence(sync, f);
+   if (r) {
+   dma_fence_put(f);
+   return r;
+   }
+   }
+   return 0;
+}
+
  /**
   * amdgpu_sync_vm_fence - remember to sync to this VM fence
   *
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
index 7c0fe20c470d..b142175b65b6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
@@ -48,6 +48,7 @@ struct amdgpu_sync {
  
  void amdgpu_sync_create(struct amdgpu_sync *sync);

  int amdgpu_sync_fence(struct amdgpu_sync *sync, struct dma_fence *f);
+int amdgpu_sync_fence_chain(struct amdgpu_sync *sync, struct dma_fence *f);
  int amdgpu_sync_vm_fence(struct amdgpu_sync *sync, struct dma_fence *fence);
  int amdgpu_sync_resv(struct amdgpu_device *adev, struct amdgpu_sync *sync,
 struct dma_resv *resv, enum amdgpu_sync_mode mode,


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 00/40] [Set 8] Rid W=1 warnings from GPU

[Christian König]

Only skimmed over them, but over all looks sane to me.

Series is Acked-by: Christian König 

Thanks,
Christian.

Am 23.11.20 um 12:18 schrieb Lee Jones:

This set is part of a larger effort attempting to clean-up W=1
kernel builds, which are currently overwhelmingly riddled with
niggly little warnings.

Only 900 (from 5000) to go!

Lee Jones (40):
   drm/radeon/radeon_device: Consume our own header where the prototypes
 are located
   drm/amd/amdgpu/amdgpu_ttm: Add description for 'page_flags'
   drm/amd/amdgpu/amdgpu_ib: Provide docs for 'amdgpu_ib_schedule()'s
 'job' param
   drm/amd/amdgpu/amdgpu_virt: Correct possible copy/paste or doc-rot
 misnaming issue
   drm/amd/amdgpu/cik_ih: Supply description for 'ih' in
 'cik_ih_{get,set}_wptr()'
   drm/amd/amdgpu/uvd_v4_2: Fix some kernel-doc misdemeanours
   drm/amd/amdgpu/dce_v8_0: Supply description for 'async'
   drm/amd/amdgpu/cik_sdma: Supply some missing function param
 descriptions
   drm/amd/amdgpu/gfx_v7_0: Clean-up a bunch of kernel-doc related issues
   drm/msm/disp/dpu1/dpu_core_perf: Fix kernel-doc formatting issues
   drm/msm/disp/dpu1/dpu_hw_blk: Add one missing and remove an extra
 param description
   drm/msm/disp/dpu1/dpu_formats: Demote non-conformant kernel-doc header
   drm/msm/disp/dpu1/dpu_hw_catalog: Remove duplicated initialisation of
 'max_linewidth'
   drm/msm/disp/dpu1/dpu_hw_catalog: Move definitions to the only place
 they are used
   drm/nouveau/nvkm/subdev/bios/init: Demote obvious abuse of kernel-doc
   drm/amd/amdgpu/si_dma: Fix a bunch of function documentation issues
   drm/amd/amdgpu/gfx_v6_0: Supply description for
 'gfx_v6_0_ring_test_ib()'s 'timeout' param
   drm/msm/disp/dpu1/dpu_encoder: Fix a few parameter/member formatting
 issues
   drm/msm/disp/dpu1/dpu_hw_lm: Fix misnaming of parameter 'ctx'
   drm/msm/disp/dpu1/dpu_hw_sspp: Fix kernel-doc formatting abuse
   drm/amd/amdgpu/uvd_v3_1: Fix-up some documentation issues
   drm/amd/amdgpu/dce_v6_0: Fix formatting and missing parameter
 description issues
   drm/amd/include/vega20_ip_offset: Mark top-level IP_BASE definition as
 __maybe_unused
   drm/amd/include/navi10_ip_offset: Mark top-level IP_BASE as
 __maybe_unused
   drm/amd/include/arct_ip_offset: Mark top-level IP_BASE definition as
 __maybe_unused
   drm/amd/include/navi14_ip_offset: Mark top-level IP_BASE as
 __maybe_unused
   drm/amd/include/navi12_ip_offset: Mark top-level IP_BASE as
 __maybe_unused
   drm/amd/include/sienna_cichlid_ip_offset: Mark top-level IP_BASE as
 __maybe_unused
   drm/amd/include/vangogh_ip_offset: Mark top-level IP_BASE as
 __maybe_unused
   drm/amd/include/dimgrey_cavefish_ip_offset: Mark top-level IP_BASE as
 __maybe_unused
   drm/msm/disp/dpu1/dpu_rm: Fix formatting issues and supply
 'global_state' description
   drm/msm/disp/dpu1/dpu_vbif: Fix a couple of function param
 descriptions
   drm/amd/amdgpu/cik_sdma: Add one and remove another function param
 description
   drm/amd/amdgpu/uvd_v4_2: Add one and remove another function param
 description
   drm/msm/disp/dpu1/dpu_plane: Fix some spelling and missing function
 param descriptions
   drm/amd/amdgpu/gmc_v7_0: Add some missing kernel-doc descriptions
   drm/amd/amdgpu/gmc_v8_0: Fix more issues attributed to copy/paste
   drm/msm/msm_drv: Make '_msm_ioremap()' static
   drm/amd/amdgpu/gmc_v9_0: Remove unused table
 'ecc_umc_mcumc_status_addrs'
   drm/amd/amdgpu/gmc_v9_0: Suppy some missing function doc descriptions

  drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c|   1 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c   |   1 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c  |  12 +-
  drivers/gpu/drm/amd/amdgpu/cik_ih.c   |   2 +
  drivers/gpu/drm/amd/amdgpu/cik_sdma.c |  18 ++-
  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c |   2 +-
  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c |   1 +
  drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c |   1 +
  drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c |  33 +++--
  drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c |   7 +-
  drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c |   5 +
  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c |  38 +
  drivers/gpu/drm/amd/amdgpu/si_dma.c   |  14 +-
  drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c |  10 +-
  drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c |  10 +-
  drivers/gpu/drm/amd/include/arct_ip_offset.h  |   4 +-
  .../amd/include/dimgrey_cavefish_ip_offset.h  |   2 +-
  .../gpu/drm/amd/include/navi10_ip_offset.h|   2 +-
  .../gpu/drm/amd/include/navi12_ip_offset.h|   2 +-
  .../gpu/drm/amd/include/navi14_ip_offset.h|   2 +-
  .../amd/include/sienna_cichlid_ip_offset.h|   2 +-
  .../gpu/drm/amd/include/vangogh_ip_offset.h   |   2 +-
  .../gpu/drm/amd/include/vega20_ip_offset.h|   2 +-
  drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c |  17 +--
  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   |  15 +-
  

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[Miguel Ojeda]

On Sun, Nov 22, 2020 at 7:22 PM James Bottomley
 wrote:
>
> Well, it's a problem in an error leg, sure, but it's not a really
> compelling reason for a 141 patch series, is it?  All that fixing this
> error will do is get the driver to print "oh dear there's a problem"
> under four more conditions than it previously did.
>
> We've been at this for three years now with nearly a thousand patches,
> firstly marking all the fall throughs with /* fall through */ and later
> changing it to fallthrough.  At some point we do have to ask if the
> effort is commensurate with the protection afforded.  Please tell me
> our reward for all this effort isn't a single missing error print.

It isn't that much effort, isn't it? Plus we need to take into account
the future mistakes that it might prevent, too. So even if there were
zero problems found so far, it is still a positive change.

I would agree if these changes were high risk, though; but they are
almost trivial.

Cheers,
Miguel
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] MAINTAINERS tag for cleanup robot

[Joe Perches]

On Sat, 2020-11-21 at 09:18 -0800, James Bottomley wrote:
> On Sat, 2020-11-21 at 08:50 -0800, t...@redhat.com wrote:
> > A difficult part of automating commits is composing the subsystem
> > preamble in the commit log.  For the ongoing effort of a fixer
> > producing one or two fixes a release the use of 'treewide:' does
> > not seem appropriate.
> > 
> > It would be better if the normal prefix was used.  Unfortunately
> > normal is not consistent across the tree.
> > 
> > D: Commit subsystem prefix
> > 
> > ex/ for FPGA DFL DRIVERS
> > 
> > D: fpga: dfl:
> 
> I've got to bet this is going to cause more issues than it solves. 
> SCSI uses scsi: : for drivers but not every driver has a
> MAINTAINERS entry.  We use either scsi: or scsi: core: for mid layer
> things, but we're not consistent.  Block uses blk-: for all
> of it's stuff but almost no s have a MAINTAINERS entry.  So
> the next thing you're going to cause is an explosion of suggested
> MAINTAINERs entries.

As well as some changes require simultaneous changes across
multiple subsystems.

> Has anyone actually complained about treewide:?

It depends on what you mean by treewide:

If a treewide: patch is applied by some "higher level" maintainer,
then generally, no.

If the treewide patch is also cc'd to many individual maintainers,
then yes, many many times.

Mostly because patches cause what is in their view churn or that
changes are not specific to their subsystem grounds.

The treewide patch is sometimes dropped, sometimes broken up and
generally not completely applied.

What would be useful in many cases like this is for a pre and post
application of the treewide patch to be compiled and the object
code verified for lack of any logic change.

Unfortunately, gcc does not guarantee deterministic compilation so
this isn't feasible with at least gcc.  Does clang guarantee this?

I'm not sure it's possible:
https://blog.llvm.org/2019/11/deterministic-builds-with-clang-and-lld.html


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] MAINTAINERS tag for cleanup robot

[James Bottomley]

On Sun, 2020-11-22 at 08:10 -0800, Tom Rix wrote:
> On 11/22/20 6:56 AM, Matthew Wilcox wrote:
> > On Sun, Nov 22, 2020 at 06:46:46AM -0800, Tom Rix wrote:
> > > On 11/21/20 7:23 PM, Matthew Wilcox wrote:
> > > > On Sat, Nov 21, 2020 at 08:50:58AM -0800, t...@redhat.com
> > > > wrote:
> > > > > The fixer review is
> > > > > https://reviews.llvm.org/D91789
> > > > > 
> > > > > A run over allyesconfig for x86_64 finds 62 issues, 5 are
> > > > > false positives. The false positives are caused by macros
> > > > > passed to other macros and by some macro expansions that did
> > > > > not have an extra semicolon.
> > > > > 
> > > > > This cleans up about 1,000 of the current 10,000 -Wextra-
> > > > > semi-stmt warnings in linux-next.
> > > > Are any of them not false-positives?  It's all very well to
> > > > enable stricter warnings, but if they don't fix any bugs,
> > > > they're just churn.
> > > > 
> > > While enabling additional warnings may be a side effect of this
> > > effort
> > > 
> > > the primary goal is to set up a cleaning robot. After that a
> > > refactoring robot.
> > Why do we need such a thing?  Again, it sounds like more churn.
> > It's really annoying when I'm working on something important that
> > gets derailed by pointless churn.  Churn also makes it harder to
> > backport patches to earlier kernels.
> > 
> A refactoring example on moving to treewide, consistent use of a new
> api may help.
> 
> Consider
> 
> 2efc459d06f1630001e3984854848a5647086232
> 
> sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output
> 
> A new api for printing in the sysfs.  How do we use it treewide ?
> 
> Done manually, it would be a heroic effort requiring high level
> maintainers pushing and likely only get partially done.
> 
> If a refactoring programatic fixit is done and validated on a one
> subsystem, it can run on all the subsystems.
> 
> The effort is a couple of weeks to write and validate the fixer,
> hours to run over the tree.
> 
> It won't be perfect but will be better than doing it manually.

Here's a thought: perhaps we don't.  sysfs_emit isn't a "new api" its a
minor rewrap of existing best practice.  The damage caused by the churn
of forcing its use everywhere would far outweigh any actual benefit
because pretty much every bug in this area has already been caught and
killed by existing tools.  We can enforce sysfs_emit going forwards
using tools like checkpatch but there's no benefit and a lot of harm to
be done by trying to churn the entire tree retrofitting it (both in
terms of review time wasted as well as patch series derailed).

James


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[James Bottomley]

On Sun, 2020-11-22 at 10:25 -0800, Joe Perches wrote:
> On Sun, 2020-11-22 at 10:21 -0800, James Bottomley wrote:
> > Please tell me our reward for all this effort isn't a single
> > missing error print.
> 
> There were quite literally dozens of logical defects found
> by the fallthrough additions.  Very few were logging only.

So can you give us the best examples (or indeed all of them if someone
is keeping score)?  hopefully this isn't a US election situation ...

James


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] MAINTAINERS tag for cleanup robot

[Joe Perches]

On Sat, 2020-11-21 at 08:50 -0800, t...@redhat.com wrote:
> A difficult part of automating commits is composing the subsystem
> preamble in the commit log.  For the ongoing effort of a fixer producing
> one or two fixes a release the use of 'treewide:' does not seem appropriate.
> 
> It would be better if the normal prefix was used.  Unfortunately normal is
> not consistent across the tree.
> 
> So I am looking for comments for adding a new tag to the MAINTAINERS file
> 
>   D: Commit subsystem prefix
> 
> ex/ for FPGA DFL DRIVERS
> 
>   D: fpga: dfl:

I'm all for it.  Good luck with the effort.  It's not completely trivial.

>From a decade ago:

https://lore.kernel.org/lkml/1289919077.28741.50.camel@Joe-Laptop/

(and that thread started with extra semicolon patches too)

> Continuing with cleaning up clang's -Wextra-semi-stmt

> diff --git a/Makefile b/Makefile
[]
> @@ -1567,20 +1567,21 @@ help:
>    echo  ''
>   @echo  'Static analysers:'
>   @echo  '  checkstack  - Generate a list of stack hogs'
>   @echo  '  versioncheck- Sanity check on version.h usage'
>   @echo  '  includecheck- Check for duplicate included header files'
>   @echo  '  export_report   - List the usages of all exported symbols'
>   @echo  '  headerdep   - Detect inclusion cycles in headers'
>   @echo  '  coccicheck  - Check with Coccinelle'
>   @echo  '  clang-analyzer  - Check with clang static analyzer'
>   @echo  '  clang-tidy  - Check with clang-tidy'
> + @echo  '  clang-tidy-fix  - Check and fix with clang-tidy'

A pity the ordering of the code below isn't the same as the above.

> -PHONY += clang-tidy clang-analyzer
> +PHONY += clang-tidy-fix clang-tidy clang-analyzer
[]
> -clang-tidy clang-analyzer: $(extmod-prefix)compile_commands.json
> +clang-tidy-fix clang-tidy clang-analyzer: 
> $(extmod-prefix)compile_commands.json
>   $(call cmd,clang_tools)
>  else
> -clang-tidy clang-analyzer:
> +clang-tidy-fix clang-tidy clang-analyzer:

[]

> diff --git a/scripts/clang-tools/run-clang-tools.py 
> b/scripts/clang-tools/run-clang-tools.py
[]
> @@ -22,43 +22,57 @@ def parse_arguments():
[]
>  parser.add_argument("type",
> -choices=["clang-tidy", "clang-analyzer"],
> +choices=["clang-tidy-fix", "clang-tidy", 
> "clang-analyzer"],

etc...

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] MAINTAINERS tag for cleanup robot

[Joe Perches]

On Sun, 2020-11-22 at 08:49 -0800, James Bottomley wrote:
> We can enforce sysfs_emit going forwards
> using tools like checkpatch

It's not really possible for checkpatch to find or warn about
sysfs uses of sprintf. checkpatch is really just a trivial
line-by-line parser and it has no concept of code intent.

It just can't warn on every use of the sprintf family.
There are just too many perfectly valid uses.

> but there's no benefit and a lot of harm to
> be done by trying to churn the entire tree

Single uses of sprintf for sysfs is not really any problem.

But likely there are still several possible overrun sprintf/snprintf
paths in sysfs.  Some of them are very obscure and unlikely to be
found by a robot as the logic for sysfs buf uses can be fairly twisty.

But provably correct conversions IMO _should_ be done and IMO churn
considerations should generally have less importance.



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[Joe Perches]

On Sun, 2020-11-22 at 10:21 -0800, James Bottomley wrote:
> Please tell me
> our reward for all this effort isn't a single missing error print.

There were quite literally dozens of logical defects found
by the fallthrough additions.  Very few were logging only.



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] MAINTAINERS tag for cleanup robot

[Tom Rix]

On 11/21/20 7:23 PM, Matthew Wilcox wrote:
> On Sat, Nov 21, 2020 at 08:50:58AM -0800, t...@redhat.com wrote:
>> The fixer review is
>> https://reviews.llvm.org/D91789
>>
>> A run over allyesconfig for x86_64 finds 62 issues, 5 are false positives.
>> The false positives are caused by macros passed to other macros and by
>> some macro expansions that did not have an extra semicolon.
>>
>> This cleans up about 1,000 of the current 10,000 -Wextra-semi-stmt
>> warnings in linux-next.
> Are any of them not false-positives?  It's all very well to enable
> stricter warnings, but if they don't fix any bugs, they're just churn.
>
While enabling additional warnings may be a side effect of this effort

the primary goal is to set up a cleaning robot. After that a refactoring robot.

Tom

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[Joe Perches]

On Sun, 2020-11-22 at 11:12 -0800, James Bottomley wrote:
> On Sun, 2020-11-22 at 10:25 -0800, Joe Perches wrote:
> > On Sun, 2020-11-22 at 10:21 -0800, James Bottomley wrote:
> > > Please tell me our reward for all this effort isn't a single
> > > missing error print.
> > 
> > There were quite literally dozens of logical defects found
> > by the fallthrough additions.  Very few were logging only.
> 
> So can you give us the best examples (or indeed all of them if someone
> is keeping score)?  hopefully this isn't a US election situation ...

Gustavo?  Are you running for congress now?

https://lwn.net/Articles/794944/


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] MAINTAINERS tag for cleanup robot

[James Bottomley]

On Sat, 2020-11-21 at 08:50 -0800, t...@redhat.com wrote:
> A difficult part of automating commits is composing the subsystem
> preamble in the commit log.  For the ongoing effort of a fixer
> producing
> one or two fixes a release the use of 'treewide:' does not seem
> appropriate.
> 
> It would be better if the normal prefix was used.  Unfortunately
> normal is
> not consistent across the tree.
> 
> 
>   D: Commit subsystem prefix
> 
> ex/ for FPGA DFL DRIVERS
> 
>   D: fpga: dfl:
> 

I've got to bet this is going to cause more issues than it solves. 
SCSI uses scsi: : for drivers but not every driver has a
MAINTAINERS entry.  We use either scsi: or scsi: core: for mid layer
things, but we're not consistent.  Block uses blk-: for all
of it's stuff but almost no s have a MAINTAINERS entry.  So
the next thing you're going to cause is an explosion of suggested
MAINTAINERs entries.

Has anyone actually complained about treewide:?

James


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[James Bottomley]

On Sun, 2020-11-22 at 11:22 -0800, Joe Perches wrote:
> On Sun, 2020-11-22 at 11:12 -0800, James Bottomley wrote:
> > On Sun, 2020-11-22 at 10:25 -0800, Joe Perches wrote:
> > > On Sun, 2020-11-22 at 10:21 -0800, James Bottomley wrote:
> > > > Please tell me our reward for all this effort isn't a single
> > > > missing error print.
> > > 
> > > There were quite literally dozens of logical defects found
> > > by the fallthrough additions.  Very few were logging only.
> > 
> > So can you give us the best examples (or indeed all of them if
> > someone is keeping score)?  hopefully this isn't a US election
> > situation ...
> 
> Gustavo?  Are you running for congress now?
> 
> https://lwn.net/Articles/794944/

That's 21 reported fixes of which about 50% seem to produce no change
in code behaviour at all, a quarter seem to have no user visible effect
with the remaining quarter producing unexpected errors on obscure
configuration parameters, which is why no-one really noticed them
before.

James


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[Finn Thain]

On Sun, 22 Nov 2020, Miguel Ojeda wrote:

> 
> It isn't that much effort, isn't it? Plus we need to take into account 
> the future mistakes that it might prevent, too.

We should also take into account optimisim about future improvements in 
tooling.

> So even if there were zero problems found so far, it is still a positive 
> change.
> 

It is if you want to spin it that way.

> I would agree if these changes were high risk, though; but they are 
> almost trivial.
> 

This is trivial:

 case 1:
this();
+   fallthrough;
 case 2:
that();

But what we inevitably get is changes like this:

 case 3:
this();
+   break;
 case 4:
hmmm();

Why? Mainly to silence the compiler. Also because the patch author argued 
successfully that they had found a theoretical bug, often in mature code.

But is anyone keeping score of the regressions? If unreported bugs count, 
what about unreported regressions?

> Cheers,
> Miguel
> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] MAINTAINERS tag for cleanup robot

[Tom Rix]

On 11/21/20 9:10 AM, Joe Perches wrote:
> On Sat, 2020-11-21 at 08:50 -0800, t...@redhat.com wrote:
>> A difficult part of automating commits is composing the subsystem
>> preamble in the commit log.  For the ongoing effort of a fixer producing
>> one or two fixes a release the use of 'treewide:' does not seem appropriate.
>>
>> It would be better if the normal prefix was used.  Unfortunately normal is
>> not consistent across the tree.
>>
>> So I am looking for comments for adding a new tag to the MAINTAINERS file
>>
>>  D: Commit subsystem prefix
>>
>> ex/ for FPGA DFL DRIVERS
>>
>>  D: fpga: dfl:
> I'm all for it.  Good luck with the effort.  It's not completely trivial.
>
> From a decade ago:
>
> https://lore.kernel.org/lkml/1289919077.28741.50.camel@Joe-Laptop/
>
> (and that thread started with extra semicolon patches too)

Reading the history, how about this.

get_mataintainer.pl outputs a single prefix, if multiple files have the same 
prefix it works, if they don't its an error.

Another script 'commit_one_file.sh' does the call to get_mainainter.pl to get 
the prefix and be called by run-clang-tools.py to get the fixer specific 
message.

Defer minimizing the commits by combining similar subsystems till later.

In a steady state case, this should be uncommon.


>
>> Continuing with cleaning up clang's -Wextra-semi-stmt
>> diff --git a/Makefile b/Makefile
> []
>> @@ -1567,20 +1567,21 @@ help:
>>   echo  ''
>>  @echo  'Static analysers:'
>>  @echo  '  checkstack  - Generate a list of stack hogs'
>>  @echo  '  versioncheck- Sanity check on version.h usage'
>>  @echo  '  includecheck- Check for duplicate included header files'
>>  @echo  '  export_report   - List the usages of all exported symbols'
>>  @echo  '  headerdep   - Detect inclusion cycles in headers'
>>  @echo  '  coccicheck  - Check with Coccinelle'
>>  @echo  '  clang-analyzer  - Check with clang static analyzer'
>>  @echo  '  clang-tidy  - Check with clang-tidy'
>> +@echo  '  clang-tidy-fix  - Check and fix with clang-tidy'
> A pity the ordering of the code below isn't the same as the above.

Taken care thanks!

Tom


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[James Bottomley]

On Sun, 2020-11-22 at 08:17 -0800, Kees Cook wrote:
> On Fri, Nov 20, 2020 at 11:51:42AM -0800, Jakub Kicinski wrote:
> > On Fri, 20 Nov 2020 11:30:40 -0800 Kees Cook wrote:
> > > On Fri, Nov 20, 2020 at 10:53:44AM -0800, Jakub Kicinski wrote:
> > > > On Fri, 20 Nov 2020 12:21:39 -0600 Gustavo A. R. Silva wrote:  
> > > > > This series aims to fix almost all remaining fall-through
> > > > > warnings in order to enable -Wimplicit-fallthrough for Clang.
> > > > > 
> > > > > In preparation to enable -Wimplicit-fallthrough for Clang,
> > > > > explicitly add multiple break/goto/return/fallthrough
> > > > > statements instead of just letting the code fall through to
> > > > > the next case.
> > > > > 
> > > > > Notice that in order to enable -Wimplicit-fallthrough for
> > > > > Clang, this change[1] is meant to be reverted at some point.
> > > > > So, this patch helps to move in that direction.
> > > > > 
> > > > > Something important to mention is that there is currently a
> > > > > discrepancy between GCC and Clang when dealing with switch
> > > > > fall-through to empty case statements or to cases that only
> > > > > contain a break/continue/return statement[2][3][4].  
> > > > 
> > > > Are we sure we want to make this change? Was it discussed
> > > > before?
> > > > 
> > > > Are there any bugs Clangs puritanical definition of fallthrough
> > > > helped find?
> > > > 
> > > > IMVHO compiler warnings are supposed to warn about issues that
> > > > could be bugs. Falling through to default: break; can hardly be
> > > > a bug?!  
> > > 
> > > It's certainly a place where the intent is not always clear. I
> > > think this makes all the cases unambiguous, and doesn't impact
> > > the machine code, since the compiler will happily optimize away
> > > any behavioral redundancy.
> > 
> > If none of the 140 patches here fix a real bug, and there is no
> > change to machine code then it sounds to me like a W=2 kind of a
> > warning.
> 
> FWIW, this series has found at least one bug so far:
> https://lore.kernel.org/lkml/CAFCwf11izHF=g1mGry1fE5kvFFFrxzhPSM6qKAO8gxSp=kr...@mail.gmail.com/


Well, it's a problem in an error leg, sure, but it's not a really
compelling reason for a 141 patch series, is it?  All that fixing this
error will do is get the driver to print "oh dear there's a problem"
under four more conditions than it previously did.

We've been at this for three years now with nearly a thousand patches,
firstly marking all the fall throughs with /* fall through */ and later
changing it to fallthrough.  At some point we do have to ask if the
effort is commensurate with the protection afforded.  Please tell me
our reward for all this effort isn't a single missing error print.

James


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[James Bottomley]

On Mon, 2020-11-23 at 09:54 +1100, Finn Thain wrote:
> But is anyone keeping score of the regressions? If unreported bugs
> count, what about unreported regressions?

Well, I was curious about the former (obviously no tool will tell me
about the latter), so I asked git what patches had a fall-through
series named in a fixes tag and these three popped out:

9cf51446e686 bpf, powerpc: Fix misuse of fallthrough in bpf_jit_comp()
6a9dc5fd6170 lib: Revert use of fallthrough pseudo-keyword in lib/
91dbd73a1739 mips/oprofile: Fix fallthrough placement

I don't think any of these is fixing a significant problem, but they
did cause someone time and trouble to investigate.

James


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] MAINTAINERS tag for cleanup robot

[Joe Perches]

On Mon, 2020-11-23 at 09:33 +1100, Finn Thain wrote:
> On Sun, 22 Nov 2020, Joe Perches wrote:

> > But provably correct conversions IMO _should_ be done and IMO churn 
> > considerations should generally have less importance.
[]
> Moreover, the patch review workload for skilled humans is being generated 
> by the automation, which is completely backwards: the machine is supposed 
> to be helping.

Which is why the provably correct matters.

coccinelle transforms can be, but are not necessarily, provably correct.

The _show transforms done via the sysfs_emit_dev.cocci script are correct
as in commit aa838896d87a ("drivers core: Use sysfs_emit and sysfs_emit_at
for show(device *...) functions")

Worthwhile?  A different question, but I think yes as it reduces the
overall question space of the existing other sprintf overrun possibilities.


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[James Bottomley]

On Sun, 2020-11-22 at 21:35 +0100, Miguel Ojeda wrote:
> On Sun, Nov 22, 2020 at 7:22 PM James Bottomley
>  wrote:
> > Well, it's a problem in an error leg, sure, but it's not a really
> > compelling reason for a 141 patch series, is it?  All that fixing
> > this error will do is get the driver to print "oh dear there's a
> > problem" under four more conditions than it previously did.
> > 
> > We've been at this for three years now with nearly a thousand
> > patches, firstly marking all the fall throughs with /* fall through
> > */ and later changing it to fallthrough.  At some point we do have
> > to ask if the effort is commensurate with the protection
> > afforded.  Please tell me our reward for all this effort isn't a
> > single missing error print.
> 
> It isn't that much effort, isn't it?

Well, it seems to be three years of someone's time plus the maintainer
review time and series disruption of nearly a thousand patches.  Let's
be conservative and assume the producer worked about 30% on the series
and it takes about 5-10 minutes per patch to review, merge and for
others to rework existing series.  So let's say it's cost a person year
of a relatively junior engineer producing the patches and say 100h of
review and application time.  The latter is likely the big ticket item
because it's what we have in least supply in the kernel (even though
it's 20x vs the producer time).

>  Plus we need to take into account the future mistakes that it might
> prevent, too. So even if there were zero problems found so far, it is
> still a positive change.

Well, the question I was asking is if it's worth the cost which I've
tried to outline above.

> I would agree if these changes were high risk, though; but they are
> almost trivial.

It's not about the risk of the changes it's about the cost of
implementing them.  Even if you discount the producer time (which
someone gets to pay for, and if I were the engineering manager, I'd be
unhappy about), the review/merge/rework time is pretty significant in
exchange for six minor bug fixes.  Fine, when a new compiler warning
comes along it's certainly reasonable to see if we can benefit from it
and the fact that the compiler people think it's worthwhile is enough
evidence to assume this initially.  But at some point you have to ask
whether that assumption is supported by the evidence we've accumulated
over the time we've been using it.  And if the evidence doesn't support
it perhaps it is time to stop the experiment.

James


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] MAINTAINERS tag for cleanup robot

[Matthew Wilcox]

On Sat, Nov 21, 2020 at 08:50:58AM -0800, t...@redhat.com wrote:
> The fixer review is
> https://reviews.llvm.org/D91789
> 
> A run over allyesconfig for x86_64 finds 62 issues, 5 are false positives.
> The false positives are caused by macros passed to other macros and by
> some macro expansions that did not have an extra semicolon.
> 
> This cleans up about 1,000 of the current 10,000 -Wextra-semi-stmt
> warnings in linux-next.

Are any of them not false-positives?  It's all very well to enable
stricter warnings, but if they don't fix any bugs, they're just churn.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[RFC] MAINTAINERS tag for cleanup robot

[trix]

A difficult part of automating commits is composing the subsystem
preamble in the commit log.  For the ongoing effort of a fixer producing
one or two fixes a release the use of 'treewide:' does not seem appropriate.

It would be better if the normal prefix was used.  Unfortunately normal is
not consistent across the tree.

So I am looking for comments for adding a new tag to the MAINTAINERS file

D: Commit subsystem prefix

ex/ for FPGA DFL DRIVERS

D: fpga: dfl:

Continuing with cleaning up clang's -Wextra-semi-stmt

A significant number of warnings are caused by function like macros with
a trailing semicolon.  For example.

#define FOO(a) a++; <-- extra, unneeded semicolon
void bar() {
int v = 0;
FOO(a);
} 

Clang will warn at the FOO(a); expansion location. Instead of removing
the semicolon there,  the fixer removes semicolon from the macro
definition.  After the fixer, the code will be:

#define FOO(a) a++
void bar() {
int v = 0;
FOO(a);
} 

The fixer review is
https://reviews.llvm.org/D91789

A run over allyesconfig for x86_64 finds 62 issues, 5 are false positives.
The false positives are caused by macros passed to other macros and by
some macro expansions that did not have an extra semicolon.

This cleans up about 1,000 of the current 10,000 -Wextra-semi-stmt
warnings in linux-next.

An update to [RFC] clang tooling cleanup
This change adds the clang-tidy-fix as a top level target and
uses it to do the cleaning.  The next iteration will do a loop of
cleaners.  This will mean breaking clang-tidy-fix out into its own
processing function 'run_fixers'.

Makefile: Add toplevel target clang-tidy-fix to makefile

Calls clang-tidy with -fix option for a set of checkers that
programatically fixes the kernel source in place, treewide.

Signed-off-by: Tom Rix 
---
 Makefile   |  7 ---
 scripts/clang-tools/run-clang-tools.py | 20 +---
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index 47a8add4dd28..57756dbb767b 100644
--- a/Makefile
+++ b/Makefile
@@ -1567,20 +1567,21 @@ help:
 echo  ''
@echo  'Static analysers:'
@echo  '  checkstack  - Generate a list of stack hogs'
@echo  '  versioncheck- Sanity check on version.h usage'
@echo  '  includecheck- Check for duplicate included header files'
@echo  '  export_report   - List the usages of all exported symbols'
@echo  '  headerdep   - Detect inclusion cycles in headers'
@echo  '  coccicheck  - Check with Coccinelle'
@echo  '  clang-analyzer  - Check with clang static analyzer'
@echo  '  clang-tidy  - Check with clang-tidy'
+   @echo  '  clang-tidy-fix  - Check and fix with clang-tidy'
@echo  ''
@echo  'Tools:'
@echo  '  nsdeps  - Generate missing symbol namespace 
dependencies'
@echo  ''
@echo  'Kernel selftest:'
@echo  '  kselftest - Build and run kernel selftest'
@echo  '  Build, install, and boot kernel before'
@echo  '  running kselftest on it'
@echo  '  Run as root for full coverage'
@echo  '  kselftest-all - Build kernel selftest'
@@ -1842,30 +1843,30 @@ nsdeps: modules
 quiet_cmd_gen_compile_commands = GEN $@
   cmd_gen_compile_commands = $(PYTHON3) $< -a $(AR) -o $@ $(filter-out $<, 
$(real-prereqs))
 
 $(extmod-prefix)compile_commands.json: 
scripts/clang-tools/gen_compile_commands.py \
$(if $(KBUILD_EXTMOD),,$(KBUILD_VMLINUX_OBJS) $(KBUILD_VMLINUX_LIBS)) \
$(if $(CONFIG_MODULES), $(MODORDER)) FORCE
$(call if_changed,gen_compile_commands)
 
 targets += $(extmod-prefix)compile_commands.json
 
-PHONY += clang-tidy clang-analyzer
+PHONY += clang-tidy-fix clang-tidy clang-analyzer
 
 ifdef CONFIG_CC_IS_CLANG
 quiet_cmd_clang_tools = CHECK   $<
   cmd_clang_tools = $(PYTHON3) 
$(srctree)/scripts/clang-tools/run-clang-tools.py $@ $<
 
-clang-tidy clang-analyzer: $(extmod-prefix)compile_commands.json
+clang-tidy-fix clang-tidy clang-analyzer: $(extmod-prefix)compile_commands.json
$(call cmd,clang_tools)
 else
-clang-tidy clang-analyzer:
+clang-tidy-fix clang-tidy clang-analyzer:
@echo "$@ requires CC=clang" >&2
@false
 endif
 
 # Scripts to check various things for consistency
 # ---
 
 PHONY += includecheck versioncheck coccicheck export_report
 
 includecheck:
diff --git a/scripts/clang-tools/run-clang-tools.py 
b/scripts/clang-tools/run-clang-tools.py
index fa7655c7cec0..c177ca822c56 100755
--- a/scripts/clang-tools/run-clang-tools.py
+++ b/scripts/clang-tools/run-clang-tools.py
@@ -22,43 +22,57 @@ def parse_arguments():
 Returns:
 args: Dict of parsed args
 Has keys: [path, type]
 """
 usage = """Run 

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[Kees Cook]

On Fri, Nov 20, 2020 at 11:51:42AM -0800, Jakub Kicinski wrote:
> On Fri, 20 Nov 2020 11:30:40 -0800 Kees Cook wrote:
> > On Fri, Nov 20, 2020 at 10:53:44AM -0800, Jakub Kicinski wrote:
> > > On Fri, 20 Nov 2020 12:21:39 -0600 Gustavo A. R. Silva wrote:  
> > > > This series aims to fix almost all remaining fall-through warnings in
> > > > order to enable -Wimplicit-fallthrough for Clang.
> > > > 
> > > > In preparation to enable -Wimplicit-fallthrough for Clang, explicitly
> > > > add multiple break/goto/return/fallthrough statements instead of just
> > > > letting the code fall through to the next case.
> > > > 
> > > > Notice that in order to enable -Wimplicit-fallthrough for Clang, this
> > > > change[1] is meant to be reverted at some point. So, this patch helps
> > > > to move in that direction.
> > > > 
> > > > Something important to mention is that there is currently a discrepancy
> > > > between GCC and Clang when dealing with switch fall-through to empty 
> > > > case
> > > > statements or to cases that only contain a break/continue/return
> > > > statement[2][3][4].  
> > > 
> > > Are we sure we want to make this change? Was it discussed before?
> > > 
> > > Are there any bugs Clangs puritanical definition of fallthrough helped
> > > find?
> > > 
> > > IMVHO compiler warnings are supposed to warn about issues that could
> > > be bugs. Falling through to default: break; can hardly be a bug?!  
> > 
> > It's certainly a place where the intent is not always clear. I think
> > this makes all the cases unambiguous, and doesn't impact the machine
> > code, since the compiler will happily optimize away any behavioral
> > redundancy.
> 
> If none of the 140 patches here fix a real bug, and there is no change
> to machine code then it sounds to me like a W=2 kind of a warning.

FWIW, this series has found at least one bug so far:
https://lore.kernel.org/lkml/CAFCwf11izHF=g1mGry1fE5kvFFFrxzhPSM6qKAO8gxSp=kr...@mail.gmail.com/

-- 
Kees Cook
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] MAINTAINERS tag for cleanup robot

[Tom Rix]

On 11/22/20 6:56 AM, Matthew Wilcox wrote:
> On Sun, Nov 22, 2020 at 06:46:46AM -0800, Tom Rix wrote:
>> On 11/21/20 7:23 PM, Matthew Wilcox wrote:
>>> On Sat, Nov 21, 2020 at 08:50:58AM -0800, t...@redhat.com wrote:
 The fixer review is
 https://reviews.llvm.org/D91789

 A run over allyesconfig for x86_64 finds 62 issues, 5 are false positives.
 The false positives are caused by macros passed to other macros and by
 some macro expansions that did not have an extra semicolon.

 This cleans up about 1,000 of the current 10,000 -Wextra-semi-stmt
 warnings in linux-next.
>>> Are any of them not false-positives?  It's all very well to enable
>>> stricter warnings, but if they don't fix any bugs, they're just churn.
>>>
>> While enabling additional warnings may be a side effect of this effort
>>
>> the primary goal is to set up a cleaning robot. After that a refactoring 
>> robot.
> Why do we need such a thing?  Again, it sounds like more churn.
> It's really annoying when I'm working on something important that gets
> derailed by pointless churn.  Churn also makes it harder to backport
> patches to earlier kernels.
>
A refactoring example on moving to treewide, consistent use of a new api may 
help.

Consider

2efc459d06f1630001e3984854848a5647086232

sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output

A new api for printing in the sysfs.  How do we use it treewide ?

Done manually, it would be a heroic effort requiring high level maintainers 
pushing and likely only get partially done.

If a refactoring programatic fixit is done and validated on a one subsystem, it 
can run on all the subsystems.

The effort is a couple of weeks to write and validate the fixer, hours to run 
over the tree.

It won't be perfect but will be better than doing it manually.

Tom

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] MAINTAINERS tag for cleanup robot

[Finn Thain]

On Sun, 22 Nov 2020, Joe Perches wrote:

> On Sun, 2020-11-22 at 08:49 -0800, James Bottomley wrote:
> > We can enforce sysfs_emit going forwards
> > using tools like checkpatch
> 
> It's not really possible for checkpatch to find or warn about
> sysfs uses of sprintf. checkpatch is really just a trivial
> line-by-line parser and it has no concept of code intent.
> 

Checkpatch does suffer from the limitations of regular expressions. But 
that doesn't stop people from using it. Besides, Coccinelle can do 
analyses that can't be done with regular expressions, so it's moot.

> It just can't warn on every use of the sprintf family.
> There are just too many perfectly valid uses.
> 
> > but there's no benefit and a lot of harm to
> > be done by trying to churn the entire tree
> 
> Single uses of sprintf for sysfs is not really any problem.
> 
> But likely there are still several possible overrun sprintf/snprintf
> paths in sysfs.  Some of them are very obscure and unlikely to be
> found by a robot as the logic for sysfs buf uses can be fairly twisty.
> 

Logic errors of this kind are susceptible to fuzzing, formal methods, 
symbolic execution etc. No doubt there are other techniques that I don't 
know about.

> But provably correct conversions IMO _should_ be done and IMO churn 
> considerations should generally have less importance.
> 

Provably equivalent conversions are provably churn. So apparently you're 
advocating changes that are not provably equivalent.

These are patches for code not that's not been shown to be buggy. Code 
which, after patching, can be shown to be free of a specific kind of 
theoretical bug. Hardly "provably correct".

The problem is, the class of theoretical bugs that can be avoided in this 
way is probably limitless, as is the review cost and the risk of 
accidental regressions. And the payoff is entirely theoretical.

Moreover, the patch review workload for skilled humans is being generated 
by the automation, which is completely backwards: the machine is supposed 
to be helping.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] MAINTAINERS tag for cleanup robot

[Matthew Wilcox]

On Sun, Nov 22, 2020 at 06:46:46AM -0800, Tom Rix wrote:
> 
> On 11/21/20 7:23 PM, Matthew Wilcox wrote:
> > On Sat, Nov 21, 2020 at 08:50:58AM -0800, t...@redhat.com wrote:
> >> The fixer review is
> >> https://reviews.llvm.org/D91789
> >>
> >> A run over allyesconfig for x86_64 finds 62 issues, 5 are false positives.
> >> The false positives are caused by macros passed to other macros and by
> >> some macro expansions that did not have an extra semicolon.
> >>
> >> This cleans up about 1,000 of the current 10,000 -Wextra-semi-stmt
> >> warnings in linux-next.
> > Are any of them not false-positives?  It's all very well to enable
> > stricter warnings, but if they don't fix any bugs, they're just churn.
> >
> While enabling additional warnings may be a side effect of this effort
> 
> the primary goal is to set up a cleaning robot. After that a refactoring 
> robot.

Why do we need such a thing?  Again, it sounds like more churn.
It's really annoying when I'm working on something important that gets
derailed by pointless churn.  Churn also makes it harder to backport
patches to earlier kernels.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] MAINTAINERS tag for cleanup robot

[Joe Perches]

On Sun, 2020-11-22 at 08:33 -0800, Tom Rix wrote:
> On 11/21/20 9:10 AM, Joe Perches wrote:
> > On Sat, 2020-11-21 at 08:50 -0800, t...@redhat.com wrote:
> > > A difficult part of automating commits is composing the subsystem
> > > preamble in the commit log.  For the ongoing effort of a fixer producing
> > > one or two fixes a release the use of 'treewide:' does not seem 
> > > appropriate.
> > > 
> > > It would be better if the normal prefix was used.  Unfortunately normal is
> > > not consistent across the tree.
> > > 
> > > So I am looking for comments for adding a new tag to the MAINTAINERS file
> > > 
> > >   D: Commit subsystem prefix
> > > 
> > > ex/ for FPGA DFL DRIVERS
> > > 
> > >   D: fpga: dfl:
> > I'm all for it.  Good luck with the effort.  It's not completely trivial.
> > 
> > From a decade ago:
> > 
> > https://lore.kernel.org/lkml/1289919077.28741.50.camel@Joe-Laptop/
> > 
> > (and that thread started with extra semicolon patches too)
> 
> Reading the history, how about this.
> 
> get_maintainer.pl outputs a single prefix, if multiple files have the
> same prefix it works, if they don't its an error.
> 
> Another script 'commit_one_file.sh' does the call to get_mainainter.pl
> to get the prefix and be called by run-clang-tools.py to get the fixer
> specific message.

It's not whether the script used is get_maintainer or any other script,
the question is really if the MAINTAINERS file is the appropriate place
to store per-subsystem patch specific prefixes.

It is.

Then the question should be how are the forms described and what is the
inheritance priority.  My preference would be to have a default of
inherit the parent base and add basename(subsystem dirname).

Commit history seems to have standardized on using colons as the separator
between the commit prefix and the subject.

A good mechanism to explore how various subsystems have uses prefixes in
the past might be something like:

$ git log --no-merges --pretty='%s' -  | \
  perl -n -e 'print substr($_, 0, rindex($_, ":") + 1) . "\n";' | \
  sort | uniq -c | sort -rn

Using 1 for commit_count and drivers/scsi for subsystem_path, the
top 40 entries are below:

About 1% don't have a colon, and there is no real consistency even
within individual drivers below scsi.  For instance, qla2xxx:

 1  814 scsi: qla2xxx:
 2  691 scsi: lpfc:
 3  389 scsi: hisi_sas:
 4  354 scsi: ufs:
 5  339 scsi:
 6  291 qla2xxx:
 7  256 scsi: megaraid_sas:
 8  249 scsi: mpt3sas:
 9  200 hpsa:
10  190 scsi: aacraid:
11  174 lpfc:
12  153 scsi: qedf:
13  144 scsi: smartpqi:
14  139 scsi: cxlflash:
15  122 scsi: core:
16  110 [SCSI] qla2xxx:
17  108 ncr5380:
18   98 scsi: hpsa:
19   97 
20   89 treewide:
21   88 mpt3sas:
22   86 scsi: libfc:
23   85 scsi: qedi:
24   84 scsi: be2iscsi:
25   81 [SCSI] qla4xxx:
26   81 hisi_sas:
27   81 block:
28   75 megaraid_sas:
29   71 scsi: sd:
30   69 [SCSI] hpsa:
31   68 cxlflash:
32   65 scsi: libsas:
33   65 scsi: fnic:
34   61 scsi: scsi_debug:
35   60 scsi: arcmsr:
36   57 be2iscsi:
37   53 atp870u:
38   51 scsi: bfa:
39   50 scsi: storvsc:
40   48 sd:


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v3 07/12] drm/sched: Prevent any job recoveries after device is unplugged.

[Christian König]

Am 23.11.20 um 06:37 schrieb Andrey Grodzovsky:


On 11/22/20 6:57 AM, Christian König wrote:

Am 21.11.20 um 06:21 schrieb Andrey Grodzovsky:

No point to try recovery if device is gone, it's meaningless.


I think that this should go into the device specific recovery 
function and not in the scheduler.



The timeout timer is rearmed here, so this prevents any new recovery 
work to restart from here
after drm_dev_unplug was executed from amdgpu_pci_remove.It will not 
cover other places like
job cleanup or starting new job but those should stop once the 
scheduler thread is stopped later.


Yeah, but this is rather unclean. We should probably return an error 
code instead if the timer should be rearmed or not.


Christian.



Andrey




Christian.



Signed-off-by: Andrey Grodzovsky 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c |  2 +-
  drivers/gpu/drm/etnaviv/etnaviv_sched.c   |  3 ++-
  drivers/gpu/drm/lima/lima_sched.c |  3 ++-
  drivers/gpu/drm/panfrost/panfrost_job.c   |  2 +-
  drivers/gpu/drm/scheduler/sched_main.c    | 15 ++-
  drivers/gpu/drm/v3d/v3d_sched.c   | 15 ++-
  include/drm/gpu_scheduler.h   |  6 +-
  7 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c

index d56f402..d0b0021 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -487,7 +487,7 @@ int amdgpu_fence_driver_init_ring(struct 
amdgpu_ring *ring,

    r = drm_sched_init(>sched, _sched_ops,
 num_hw_submission, amdgpu_job_hang_limit,
-   timeout, ring->name);
+   timeout, ring->name, >ddev);
  if (r) {
  DRM_ERROR("Failed to create scheduler on ring %s.\n",
    ring->name);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c 
b/drivers/gpu/drm/etnaviv/etnaviv_sched.c

index cd46c88..7678287 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
@@ -185,7 +185,8 @@ int etnaviv_sched_init(struct etnaviv_gpu *gpu)
    ret = drm_sched_init(>sched, _sched_ops,
   etnaviv_hw_jobs_limit, etnaviv_job_hang_limit,
- msecs_to_jiffies(500), dev_name(gpu->dev));
+ msecs_to_jiffies(500), dev_name(gpu->dev),
+ gpu->drm);
  if (ret)
  return ret;
  diff --git a/drivers/gpu/drm/lima/lima_sched.c 
b/drivers/gpu/drm/lima/lima_sched.c

index dc6df9e..8a7e5d7ca 100644
--- a/drivers/gpu/drm/lima/lima_sched.c
+++ b/drivers/gpu/drm/lima/lima_sched.c
@@ -505,7 +505,8 @@ int lima_sched_pipe_init(struct lima_sched_pipe 
*pipe, const char *name)

    return drm_sched_init(>base, _sched_ops, 1,
    lima_job_hang_limit, msecs_to_jiffies(timeout),
-  name);
+  name,
+  pipe->ldev->ddev);
  }
    void lima_sched_pipe_fini(struct lima_sched_pipe *pipe)
diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c 
b/drivers/gpu/drm/panfrost/panfrost_job.c

index 30e7b71..37b03b01 100644
--- a/drivers/gpu/drm/panfrost/panfrost_job.c
+++ b/drivers/gpu/drm/panfrost/panfrost_job.c
@@ -520,7 +520,7 @@ int panfrost_job_init(struct panfrost_device 
*pfdev)

  ret = drm_sched_init(>queue[j].sched,
   _sched_ops,
   1, 0, msecs_to_jiffies(500),
- "pan_js");
+ "pan_js", pfdev->ddev);
  if (ret) {
  dev_err(pfdev->dev, "Failed to create scheduler: %d.", 
ret);

  goto err_sched;
diff --git a/drivers/gpu/drm/scheduler/sched_main.c 
b/drivers/gpu/drm/scheduler/sched_main.c

index c3f0bd0..95db8c6 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -53,6 +53,7 @@
  #include 
  #include 
  #include 
+#include 
    #define CREATE_TRACE_POINTS
  #include "gpu_scheduler_trace.h"
@@ -283,8 +284,16 @@ static void drm_sched_job_timedout(struct 
work_struct *work)

  struct drm_gpu_scheduler *sched;
  struct drm_sched_job *job;
  +    int idx;
+
  sched = container_of(work, struct drm_gpu_scheduler, 
work_tdr.work);

  +    if (!drm_dev_enter(sched->ddev, )) {
+    DRM_INFO("%s - device unplugged skipping recovery on 
scheduler:%s",

+ __func__, sched->name);
+    return;
+    }
+
  /* Protects against concurrent deletion in 
drm_sched_get_cleanup_job */

  spin_lock(>job_list_lock);
  job = list_first_entry_or_null(>ring_mirror_list,
@@ -316,6 +325,8 @@ static void drm_sched_job_timedout(struct 
work_struct *work)

  spin_lock(>job_list_lock);
  drm_sched_start_timeout(sched);
  spin_unlock(>job_list_lock);
+
+    drm_dev_exit(idx);
  }
     /**
@@ -845,7 +856,8 @@ int drm_sched_init(struct drm_gpu_scheduler *sched,
 unsigned hw_submission,
 unsigned