Re: [PATCH] drm/amd/display: Simplify same effect if/else blocks

[Joe Perches]

On Fri, 2023-03-03 at 15:35 -0500, Harry Wentland wrote:
> Actually I was wrong. Too many similar-looking snippets in this
> function made me look at the wrong thing. This change is fine and
> Reviewed-by: Harry Wentland 

Re: [PATCH] drm/amd/display: Simplify same effect if/else blocks

[Joe Perches]

On Sun, 2023-01-15 at 15:30 +0530, Deepak R Varma wrote:
> The if / else block code has same effect irrespective of the logical
> evaluation.  Hence, simply the implementation by removing the unnecessary
> conditional evaluation. While at it, also fix the long line checkpatch
> complaint. Issue identified using cond_no_effect.cocci Coccinelle
> semantic patch script.
> 
> Signed-off-by: Deepak R Varma 
> ---
> Please note: The proposed change is compile tested only. If there are any
> inbuilt test cases that I should run for further verification, I will 
> appreciate
> guidance about it. Thank you.

Preface: I do not know the code.

Perhaps Rodrigo Siqueira made a copy/paste error submitting the code for
commit 9114b55fabae ("drm/amd/display: Fix SubVP control flow in the MPO 
context")
as the code prior to this change is identical.

Perhaps one of the false uses should be true or dependent on the
interdependent_update_lock state.

> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
> b/drivers/gpu/drm/amd/display/dc/core/dc.c
[]
> @@ -3470,14 +3470,9 @@ static void commit_planes_for_stream(struct dc *dc,
>   /* Since phantom pipe programming is moved to 
> post_unlock_program_front_end,
>* move the SubVP lock to after the phantom pipes have been 
> setup
>*/
> - if (should_lock_all_pipes && 
> dc->hwss.interdependent_update_lock) {
> - if (dc->hwss.subvp_pipe_control_lock)
> - dc->hwss.subvp_pipe_control_lock(dc, context, 
> false, should_lock_all_pipes, NULL, subvp_prev_use);
> - } else {
> - if (dc->hwss.subvp_pipe_control_lock)
> - dc->hwss.subvp_pipe_control_lock(dc, context, 
> false, should_lock_all_pipes, NULL, subvp_prev_use);
> - }
> -

Perhaps something like:

if (dc->hwss.subvp_pipe_control_lock)
dc->hwss.subvp_pipe_control_lock(dc, context,
 should_lock_all_pipes 
&&
 
dc->hwss.interdependent_update_lock,
 should_lock_all_pipes, 
NULL, subvp_prev_use);

> + if (dc->hwss.subvp_pipe_control_lock)
> + dc->hwss.subvp_pipe_control_lock(dc, context, false, 
> should_lock_all_pipes,
> +  NULL, subvp_prev_use);
>   return;
>   }
>  

Re: [PATCH 1/6] drivers: usb: remove usage of list iterator past the loop body

[Joe Perches]

On Mon, 2022-02-28 at 14:24 +0300, Dan Carpenter wrote:

> a multi-line indent gets curly braces for readability even though
> it's not required by C.  And then both sides would get curly braces.

That's more your personal preference than a coding style guideline.

Re: [PATCH 1/3] lib/string_helpers: Consolidate yesno() implementation

[Joe Perches]

On Wed, 2022-01-19 at 16:00 -0500, Steven Rostedt wrote:
> On Wed, 19 Jan 2022 21:25:08 +0200
> Andy Shevchenko  wrote:
> 
> > > I say keep it one line!
> > > 
> > > Reviewed-by: Steven Rostedt (Google)   
> > 
> > I believe Sakari strongly follows the 80 rule, which means...
> 
> Checkpatch says "100" I think we need to simply update the docs (the
> documentation always lags the code ;-)

checkpatch doesn't say anything normally, it's a stupid script.
It just mindlessly bleats a message when a line exceeds 100 chars...

Just fyi: I think it's nicer on a single line too.

Re: [PATCH] drm/amd/amdgpu: remove useless break after return

[Joe Perches]

On Sun, 2021-11-14 at 23:14 -0800, Bernard Zhao wrote:
> This change is to remove useless break after return.
[]
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c 
> b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
[]
> @@ -2092,22 +2092,18 @@ static int dce_v8_0_pick_dig_encoder(struct 
> drm_encoder *encoder)
>   return 1;
>   else
>   return 0;
> - break;
>   case ENCODER_OBJECT_ID_INTERNAL_UNIPHY1:
>   if (dig->linkb)
>   return 3;
>   else
>   return 2;
> - break;
>   case ENCODER_OBJECT_ID_INTERNAL_UNIPHY2:
>   if (dig->linkb)
>   return 5;
>   else
>   return 4;
> - break;
>   case ENCODER_OBJECT_ID_INTERNAL_UNIPHY3:
>   return 6;
> - break;
>   default:
>   DRM_ERROR("invalid encoder_id: 0x%x\n", 
> amdgpu_encoder->encoder_id);
>   return 0;

Perhaps rewrite to make the sequential numbering more obvious.
---
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 19 +++
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c 
b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index b200b9e722d97..7307524b706b4 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
@@ -2088,26 +2088,13 @@ static int dce_v8_0_pick_dig_encoder(struct drm_encoder 
*encoder)
 
switch (amdgpu_encoder->encoder_id) {
case ENCODER_OBJECT_ID_INTERNAL_UNIPHY:
-   if (dig->linkb)
-   return 1;
-   else
-   return 0;
-   break;
+   return !dig->linkb ? 0 : 1;
case ENCODER_OBJECT_ID_INTERNAL_UNIPHY1:
-   if (dig->linkb)
-   return 3;
-   else
-   return 2;
-   break;
+   return !dig->linkb ? 2 : 3;
case ENCODER_OBJECT_ID_INTERNAL_UNIPHY2:
-   if (dig->linkb)
-   return 5;
-   else
-   return 4;
-   break;
+   return !dig->linkb ? 4 : 5;
case ENCODER_OBJECT_ID_INTERNAL_UNIPHY3:
return 6;
-   break;
default:
DRM_ERROR("invalid encoder_id: 0x%x\n", 
amdgpu_encoder->encoder_id);
return 0;

Re: [PATCH 2/4] amdgpu_ucode: reduce number of pr_debug calls

[Joe Perches]

On Wed, 2021-09-29 at 19:44 -0600, Jim Cromie wrote:
> There are blocks of DRM_DEBUG calls, consolidate their args into
> single calls.  With dynamic-debug in use, each callsite consumes 56
> bytes of callsite data, and this patch removes about 65 calls, so
> it saves ~3.5kb.
> 
> no functional changes.

No functional change, but an output logging content change.

> RFC: this creates multi-line log messages, does that break any syslog
> conventions ?

It does change the output as each individual DRM_DEBUG is a call to
__drm_dbg which is effectively:

printk(KERN_DEBUG "[" DRM_NAME ":%ps] %pV",
   __builtin_return_address(0), );


> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
[]
> @@ -30,17 +30,26 @@
>  
> 
>  static void amdgpu_ucode_print_common_hdr(const struct 
> common_firmware_header *hdr)
>  {
> - DRM_DEBUG("size_bytes: %u\n", le32_to_cpu(hdr->size_bytes));
> - DRM_DEBUG("header_size_bytes: %u\n", 
> le32_to_cpu(hdr->header_size_bytes));
[]
> + DRM_DEBUG("size_bytes: %u\n"
> +   "header_size_bytes: %u\n"

etc...

Re: drm/amd/display: Simplify hdcp validate_bksv

[Joe Perches]

On Mon, 2021-07-12 at 07:02 +0800, kernel test robot wrote:
> Hi Joe,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on drm-intel/for-linux-next]
> [also build test ERROR on drm-exynos/exynos-drm-next linus/master 
> next-20210709]
> [cannot apply to kees/for-next/pstore tegra-drm/drm/tegra/for-next 
> drm/drm-next v5.13]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:
> https://github.com/0day-ci/linux/commits/Joe-Perches/drm-amd-display-Simplify-hdcp-validate_bksv/20210712-034708
> base:   git://anongit.freedesktop.org/drm-intel for-linux-next
> config: i386-randconfig-a003-20210712 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> reproduce (this is a W=1 build):
> # 
> https://github.com/0day-ci/linux/commit/66fae2c1becdcb71c95f2c6a6413de4dfe1deb51
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review 
> Joe-Perches/drm-amd-display-Simplify-hdcp-validate_bksv/20210712-034708
> git checkout 66fae2c1becdcb71c95f2c6a6413de4dfe1deb51
> # save the attached .config to linux build tree
> make W=1 ARCH=i386 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot 
> 
> All errors (new ones prefixed by >>, old ones prefixed by <<):
> 
> > > ERROR: modpost: "__popcountdi2" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] 
> > > undefined!

curious.

Anyone know why?


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

Re: [PATCH] drm/amd/display: Fix identical code for different branches

[Joe Perches]

On Sun, 2021-07-11 at 19:24 +0200, Len Baker wrote:
> The branches of the "if" statement are the same. So remove the
> unnecessary if and goto statements.
> 
> Addresses-Coverity-ID: 1456916 ("Identical code for different branches")
> Fixes: 4c283fdac08ab ("drm/amd/display: Add HDCP module")
> Signed-off-by: Len Baker 

I'm not a big fan of this type of change.

It's currently the same style used for six tests in this function
and changing this last one would just make it harder to see the
code blocks as consistent.

I doubt any reasonable compiler would produce different objects.

> diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c 
> b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c
[]
> @@ -305,10 +305,8 @@ static enum mod_hdcp_status wait_for_ready(struct 
> mod_hdcp *hdcp,
>   hdcp, "bcaps_read"))
>   goto out;
>   }
> - if (!mod_hdcp_execute_and_set(check_ksv_ready,
> - >ready_check, ,
> - hdcp, "ready_check"))
> - goto out;
> + mod_hdcp_execute_and_set(check_ksv_ready, >ready_check, ,
> +  hdcp, "ready_check");
>  out:
>   return status;
>  }
> --
> 2.25.1
> 


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

drm/amd/display: Simplify hdcp validate_bksv

[Joe Perches]

commit 06888d571b51 ("drm/amd/display: Avoid HDCP over-read and corruption")
fixed an overread with an invalid buffer length but added an unnecessary
buffer and copy.

Simplify the code by using a single uint64_t and __builtin_popcountll to
count the number of bits set in the original bksv buffer instead of a loop.

This also avoid a possible unaligned access of the temporary bksv.

Signed-off-by: Joe Perches 
---

It seems quite odd 20 bits set is a magic number here.
Should it be a specific be/le value instead?

 drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c | 11 ++-
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c 
b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c
index de872e7958b06..78a4c6dd95d99 100644
--- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c
+++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp1_execution.c
@@ -28,17 +28,10 @@
 static inline enum mod_hdcp_status validate_bksv(struct mod_hdcp *hdcp)
 {
uint64_t n = 0;
-   uint8_t count = 0;
-   u8 bksv[sizeof(n)] = { };
 
-   memcpy(bksv, hdcp->auth.msg.hdcp1.bksv, 
sizeof(hdcp->auth.msg.hdcp1.bksv));
-   n = *(uint64_t *)bksv;
+   memcpy(, hdcp->auth.msg.hdcp1.bksv, 
sizeof(hdcp->auth.msg.hdcp1.bksv));
 
-   while (n) {
-   count++;
-   n &= (n - 1);
-   }
-   return (count == 20) ? MOD_HDCP_STATUS_SUCCESS :
+   return (__builtin_popcountll(n) == 20) ? MOD_HDCP_STATUS_SUCCESS :
MOD_HDCP_STATUS_HDCP1_INVALID_BKSV;
 }
 


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

Re: [PATCH V2] treewide: Add missing semicolons to __assign_str uses

[Joe Perches]

On Sat, 2021-06-12 at 08:42 -0700, Joe Perches wrote:
> The __assign_str macro has an unusual ending semicolon but the vast
> majority of uses of the macro already have semicolon termination.

ping?


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

[PATCH V2] treewide: Add missing semicolons to __assign_str uses

[Joe Perches]

The __assign_str macro has an unusual ending semicolon but the vast
majority of uses of the macro already have semicolon termination.

$ git grep -P '\b__assign_str\b' | wc -l
551
$ git grep -P '\b__assign_str\b.*;' | wc -l
480

Add semicolons to the __assign_str() uses without semicolon termination
and all the other uses without semicolon termination via additional defines
that are equivalent to __assign_str() with the eventual goal of removing
the semicolon from the __assign_str() macro definition.

Link: 
https://lore.kernel.org/lkml/1e068d21106bb6db05b735b4916bb420e6c9842a.ca...@perches.com/

Signed-off-by: Joe Perches 
---

V2: Remove semicolon addition to #define VIF_ASSIGN as every use of
this macro already has a semicolon termination.

Compiled x84-64 allyesconfig

 drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h  | 14 
 drivers/gpu/drm/lima/lima_trace.h  |  2 +-
 drivers/infiniband/hw/hfi1/trace_misc.h|  4 +--
 drivers/infiniband/hw/hfi1/trace_rc.h  |  4 +--
 drivers/infiniband/hw/hfi1/trace_tid.h |  6 ++--
 drivers/infiniband/hw/hfi1/trace_tx.h  |  8 ++---
 drivers/infiniband/sw/rdmavt/trace_cq.h|  4 +--
 drivers/infiniband/sw/rdmavt/trace_mr.h|  2 +-
 drivers/infiniband/sw/rdmavt/trace_qp.h|  4 +--
 drivers/infiniband/sw/rdmavt/trace_rc.h|  2 +-
 drivers/infiniband/sw/rdmavt/trace_tx.h|  4 +--
 drivers/misc/mei/mei-trace.h   |  6 ++--
 .../net/ethernet/marvell/octeontx2/af/rvu_trace.h  | 12 +++
 drivers/net/fjes/fjes_trace.h  |  4 +--
 drivers/usb/cdns3/cdnsp-trace.h|  2 +-
 fs/nfs/nfs4trace.h |  6 ++--
 fs/nfs/nfstrace.h  |  4 +--
 include/trace/events/btrfs.h   |  2 +-
 include/trace/events/dma_fence.h   |  4 +--
 include/trace/events/rpcgss.h  |  4 +--
 include/trace/events/sunrpc.h  | 40 +++---
 21 files changed, 69 insertions(+), 69 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
index 0527772fe1b80..d855cb53c7e09 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
@@ -176,10 +176,10 @@ TRACE_EVENT(amdgpu_cs_ioctl,
 
TP_fast_assign(
   __entry->sched_job_id = job->base.id;
-  __assign_str(timeline, 
AMDGPU_JOB_GET_TIMELINE_NAME(job))
+  __assign_str(timeline, 
AMDGPU_JOB_GET_TIMELINE_NAME(job));
   __entry->context = 
job->base.s_fence->finished.context;
   __entry->seqno = job->base.s_fence->finished.seqno;
-  __assign_str(ring, 
to_amdgpu_ring(job->base.sched)->name)
+  __assign_str(ring, 
to_amdgpu_ring(job->base.sched)->name);
   __entry->num_ibs = job->num_ibs;
   ),
TP_printk("sched_job=%llu, timeline=%s, context=%u, seqno=%u, 
ring_name=%s, num_ibs=%u",
@@ -201,10 +201,10 @@ TRACE_EVENT(amdgpu_sched_run_job,
 
TP_fast_assign(
   __entry->sched_job_id = job->base.id;
-  __assign_str(timeline, 
AMDGPU_JOB_GET_TIMELINE_NAME(job))
+  __assign_str(timeline, 
AMDGPU_JOB_GET_TIMELINE_NAME(job));
   __entry->context = 
job->base.s_fence->finished.context;
   __entry->seqno = job->base.s_fence->finished.seqno;
-  __assign_str(ring, 
to_amdgpu_ring(job->base.sched)->name)
+  __assign_str(ring, 
to_amdgpu_ring(job->base.sched)->name);
   __entry->num_ibs = job->num_ibs;
   ),
TP_printk("sched_job=%llu, timeline=%s, context=%u, seqno=%u, 
ring_name=%s, num_ibs=%u",
@@ -229,7 +229,7 @@ TRACE_EVENT(amdgpu_vm_grab_id,
 
TP_fast_assign(
   __entry->pasid = vm->pasid;
-  __assign_str(ring, ring->name)
+  __assign_str(ring, ring->name);
   __entry->vmid = job->vmid;
   __entry->vm_hub = ring->funcs->vmhub,
   __entry->pd_addr = job->vm_pd_addr;
@@ -424,7 +424,7 @@ TRACE_EVENT(amdgpu_vm_flush,
 ),
 
TP_fast_assign(
-  __assign_str(ring, ring->name)
+  __assign_str(ring, ring->name);
   __entry->vmid = vmid;
 

[PATCH] treewide: Add missing semicolons to __assign_str uses

[Joe Perches]

The __assign_str macro has an unusual ending semicolon but the vast
majority of uses of the macro already have semicolon termination.

$ git grep -P '\b__assign_str\b' | wc -l
551
$ git grep -P '\b__assign_str\b.*;' | wc -l
480

Add semicolons to the __assign_str() uses without semicolon termination
and all the other uses without semicolon termination via additional defines
that are equivalent to __assign_str() with the eventual goal of removing
the semicolon from the __assign_str() macro definition.

Link: 
https://lore.kernel.org/lkml/1e068d21106bb6db05b735b4916bb420e6c9842a.ca...@perches.com/

Signed-off-by: Joe Perches 
---

Resending without all the direct cc's, only the mailing lists as
it seems not to have gone through via vger.

Compiled x84-64 allyesconfig

On Fri, 2021-06-04 at 12:21 -0400, Steven Rostedt wrote:
> I have no problem taking a clean up patch that adds semicolons to all
> use cases of "__assign_str()" and ever remove the one from where it is
> defined. As long as it doesn't break any builds, I'm fine with that.

Removing the semicolon from the macro definition is left for another patch.

 drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h  | 14 
 drivers/gpu/drm/lima/lima_trace.h  |  2 +-
 drivers/infiniband/hw/hfi1/trace_misc.h|  4 +--
 drivers/infiniband/hw/hfi1/trace_rc.h  |  4 +--
 drivers/infiniband/hw/hfi1/trace_tid.h |  6 ++--
 drivers/infiniband/hw/hfi1/trace_tx.h  |  8 ++---
 drivers/infiniband/sw/rdmavt/trace_cq.h|  4 +--
 drivers/infiniband/sw/rdmavt/trace_mr.h|  2 +-
 drivers/infiniband/sw/rdmavt/trace_qp.h|  4 +--
 drivers/infiniband/sw/rdmavt/trace_rc.h|  2 +-
 drivers/infiniband/sw/rdmavt/trace_tx.h|  4 +--
 drivers/misc/mei/mei-trace.h   |  6 ++--
 .../net/ethernet/marvell/octeontx2/af/rvu_trace.h  | 12 +++
 drivers/net/fjes/fjes_trace.h  |  4 +--
 drivers/usb/cdns3/cdnsp-trace.h|  2 +-
 fs/nfs/nfs4trace.h |  6 ++--
 fs/nfs/nfstrace.h  |  4 +--
 include/trace/events/btrfs.h   |  2 +-
 include/trace/events/dma_fence.h   |  4 +--
 include/trace/events/rpcgss.h  |  4 +--
 include/trace/events/sunrpc.h  | 40 +++---
 net/mac80211/trace.h   |  2 +-
 22 files changed, 70 insertions(+), 70 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
index 0527772fe1b80..d855cb53c7e09 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
@@ -176,10 +176,10 @@ TRACE_EVENT(amdgpu_cs_ioctl,
 
TP_fast_assign(
   __entry->sched_job_id = job->base.id;
-  __assign_str(timeline, 
AMDGPU_JOB_GET_TIMELINE_NAME(job))
+  __assign_str(timeline, 
AMDGPU_JOB_GET_TIMELINE_NAME(job));
   __entry->context = 
job->base.s_fence->finished.context;
   __entry->seqno = job->base.s_fence->finished.seqno;
-  __assign_str(ring, 
to_amdgpu_ring(job->base.sched)->name)
+  __assign_str(ring, 
to_amdgpu_ring(job->base.sched)->name);
   __entry->num_ibs = job->num_ibs;
   ),
TP_printk("sched_job=%llu, timeline=%s, context=%u, seqno=%u, 
ring_name=%s, num_ibs=%u",
@@ -201,10 +201,10 @@ TRACE_EVENT(amdgpu_sched_run_job,
 
TP_fast_assign(
   __entry->sched_job_id = job->base.id;
-  __assign_str(timeline, 
AMDGPU_JOB_GET_TIMELINE_NAME(job))
+  __assign_str(timeline, 
AMDGPU_JOB_GET_TIMELINE_NAME(job));
   __entry->context = 
job->base.s_fence->finished.context;
   __entry->seqno = job->base.s_fence->finished.seqno;
-  __assign_str(ring, 
to_amdgpu_ring(job->base.sched)->name)
+  __assign_str(ring, 
to_amdgpu_ring(job->base.sched)->name);
   __entry->num_ibs = job->num_ibs;
   ),
TP_printk("sched_job=%llu, timeline=%s, context=%u, seqno=%u, 
ring_name=%s, num_ibs=%u",
@@ -229,7 +229,7 @@ TRACE_EVENT(amdgpu_vm_grab_id,
 
TP_fast_assign(
   __entry->pasid = vm->pasid;
-  __assign_str(ring, ring->name)
+  __assign_str(ring, ring->name);
   __entry->vmid = job->vmid;
   __entry

[PATCH] treewide: Add missing semicolons to __assign_str uses

[Joe Perches]

The __assign_str macro has an unusual ending semicolon but the vast
majority of uses of the macro already have semicolon termination.

$ git grep -P '\b__assign_str\b' | wc -l
551
$ git grep -P '\b__assign_str\b.*;' | wc -l
480

Add semicolons to the __assign_str() uses without semicolon termination
and all the other uses without semicolon termination via additional defines
that are equivalent to __assign_str() with the eventual goal of removing
the semicolon from the __assign_str() macro definition.

Link: 
https://lore.kernel.org/lkml/1e068d21106bb6db05b735b4916bb420e6c9842a.ca...@perches.com/

Signed-off-by: Joe Perches 
---

Compiled x84-64 allyesconfig

On Fri, 2021-06-04 at 12:21 -0400, Steven Rostedt wrote:
> I have no problem taking a clean up patch that adds semicolons to all
> use cases of "__assign_str()" and ever remove the one from where it is
> defined. As long as it doesn't break any builds, I'm fine with that.

Removing the semicolon from the macro definition is left for another patch.

 drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h  | 14 
 drivers/gpu/drm/lima/lima_trace.h  |  2 +-
 drivers/infiniband/hw/hfi1/trace_misc.h|  4 +--
 drivers/infiniband/hw/hfi1/trace_rc.h  |  4 +--
 drivers/infiniband/hw/hfi1/trace_tid.h |  6 ++--
 drivers/infiniband/hw/hfi1/trace_tx.h  |  8 ++---
 drivers/infiniband/sw/rdmavt/trace_cq.h|  4 +--
 drivers/infiniband/sw/rdmavt/trace_mr.h|  2 +-
 drivers/infiniband/sw/rdmavt/trace_qp.h|  4 +--
 drivers/infiniband/sw/rdmavt/trace_rc.h|  2 +-
 drivers/infiniband/sw/rdmavt/trace_tx.h|  4 +--
 drivers/misc/mei/mei-trace.h   |  6 ++--
 .../net/ethernet/marvell/octeontx2/af/rvu_trace.h  | 12 +++
 drivers/net/fjes/fjes_trace.h  |  4 +--
 drivers/usb/cdns3/cdnsp-trace.h|  2 +-
 fs/nfs/nfs4trace.h |  6 ++--
 fs/nfs/nfstrace.h  |  4 +--
 include/trace/events/btrfs.h   |  2 +-
 include/trace/events/dma_fence.h   |  4 +--
 include/trace/events/rpcgss.h  |  4 +--
 include/trace/events/sunrpc.h  | 40 +++---
 net/mac80211/trace.h   |  2 +-
 22 files changed, 70 insertions(+), 70 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
index 0527772fe1b80..d855cb53c7e09 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
@@ -176,10 +176,10 @@ TRACE_EVENT(amdgpu_cs_ioctl,
 
TP_fast_assign(
   __entry->sched_job_id = job->base.id;
-  __assign_str(timeline, 
AMDGPU_JOB_GET_TIMELINE_NAME(job))
+  __assign_str(timeline, 
AMDGPU_JOB_GET_TIMELINE_NAME(job));
   __entry->context = 
job->base.s_fence->finished.context;
   __entry->seqno = job->base.s_fence->finished.seqno;
-  __assign_str(ring, 
to_amdgpu_ring(job->base.sched)->name)
+  __assign_str(ring, 
to_amdgpu_ring(job->base.sched)->name);
   __entry->num_ibs = job->num_ibs;
   ),
TP_printk("sched_job=%llu, timeline=%s, context=%u, seqno=%u, 
ring_name=%s, num_ibs=%u",
@@ -201,10 +201,10 @@ TRACE_EVENT(amdgpu_sched_run_job,
 
TP_fast_assign(
   __entry->sched_job_id = job->base.id;
-  __assign_str(timeline, 
AMDGPU_JOB_GET_TIMELINE_NAME(job))
+  __assign_str(timeline, 
AMDGPU_JOB_GET_TIMELINE_NAME(job));
   __entry->context = 
job->base.s_fence->finished.context;
   __entry->seqno = job->base.s_fence->finished.seqno;
-  __assign_str(ring, 
to_amdgpu_ring(job->base.sched)->name)
+  __assign_str(ring, 
to_amdgpu_ring(job->base.sched)->name);
   __entry->num_ibs = job->num_ibs;
   ),
TP_printk("sched_job=%llu, timeline=%s, context=%u, seqno=%u, 
ring_name=%s, num_ibs=%u",
@@ -229,7 +229,7 @@ TRACE_EVENT(amdgpu_vm_grab_id,
 
TP_fast_assign(
   __entry->pasid = vm->pasid;
-  __assign_str(ring, ring->name)
+  __assign_str(ring, ring->name);
   __entry->vmid = job->vmid;
   __entry->vm_hub = ring->funcs->vmhub,
   __entry->pd_addr = job->vm_pd_addr;
@@ -424,7 +

[trivial PATCH] drm/amd/display: Fix typo of format termination newline

[Joe Perches]

/n should be \n

Signed-off-by: Joe Perches 
---
 drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c   | 2 +-
 drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c | 2 +-
 drivers/gpu/drm/amd/display/dc/dcn302/dcn302_resource.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c 
b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c
index 45f96221a094..b38fee783277 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c
@@ -1724,7 +1724,7 @@ static bool init_soc_bounding_box(struct dc *dc,
DC_LOGGER_INIT(dc->ctx->logger);
 
if (!is_soc_bounding_box_valid(dc)) {
-   DC_LOG_ERROR("%s: not valid soc bounding box/n", __func__);
+   DC_LOG_ERROR("%s: not valid soc bounding box\n", __func__);
return false;
}
 
diff --git a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c 
b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
index 5b54b7fc5105..3bf66c994dd5 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c
@@ -1497,7 +1497,7 @@ static bool init_soc_bounding_box(struct dc *dc,
DC_LOGGER_INIT(dc->ctx->logger);
 
if (!is_soc_bounding_box_valid(dc)) {
-   DC_LOG_ERROR("%s: not valid soc bounding box/n", __func__);
+   DC_LOG_ERROR("%s: not valid soc bounding box\n", __func__);
return false;
}
 
diff --git a/drivers/gpu/drm/amd/display/dc/dcn302/dcn302_resource.c 
b/drivers/gpu/drm/amd/display/dc/dcn302/dcn302_resource.c
index fc2dea243d1b..84c61128423e 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn302/dcn302_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn302/dcn302_resource.c
@@ -1093,7 +1093,7 @@ static bool init_soc_bounding_box(struct dc *dc,  struct 
resource_pool *pool)
DC_LOGGER_INIT(dc->ctx->logger);
 
if (!is_soc_bounding_box_valid(dc)) {
-   DC_LOG_ERROR("%s: not valid soc bounding box/n", __func__);
+   DC_LOG_ERROR("%s: not valid soc bounding box\n", __func__);
return false;
}
 

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

Re: [PATCH] amdgpu: fix gcc -Wrestrict warning

[Joe Perches]

On Tue, 2021-03-23 at 14:04 +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann 
> 
> gcc warns about an sprintf() that uses the same buffer as source
> and destination, which is undefined behavior in C99:
> 
> drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c: In function 
> 'amdgpu_securedisplay_debugfs_write':
> drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c:141:6: error: 'sprintf' 
> argument 3 overlaps destination object 'i2c_output' [-Werror=restrict]
>   141 |  sprintf(i2c_output, "%s 0x%X", i2c_output,
>   |  ^~
>   142 |   
> securedisplay_cmd->securedisplay_out_message.send_roi_crc.i2c_buf[i]);
>   |   
> ~
> drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c:97:7: note: destination 
> object referenced by 'restrict'-qualified argument 1 was declared here
>    97 |  char i2c_output[256];
>   |   ^~
> 
> Rewrite it to remember the current offset into the buffer instead.
> 
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c
> index 834440ab9ff7..69d7f6bff5d4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c
> @@ -136,9 +136,10 @@ static ssize_t amdgpu_securedisplay_debugfs_write(struct 
> file *f, const char __u
>   ret = psp_securedisplay_invoke(psp, 
> TA_SECUREDISPLAY_COMMAND__SEND_ROI_CRC);
>   if (!ret) {
>   if (securedisplay_cmd->status == 
> TA_SECUREDISPLAY_STATUS__SUCCESS) {
> + int pos = 0;
>   memset(i2c_output,  0, sizeof(i2c_output));
>   for (i = 0; i < 
> TA_SECUREDISPLAY_I2C_BUFFER_SIZE; i++)
> - sprintf(i2c_output, "%s 0x%X", 
> i2c_output,
> + pos += sprintf(i2c_output + pos, " 
> 0x%X",
>   
> securedisplay_cmd->securedisplay_out_message.send_roi_crc.i2c_buf[i]);
>   dev_info(adev->dev, "SECUREDISPLAY: I2C buffer 
> out put is :%s\n", i2c_output);

Perhaps use a hex output like:

---
 drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c
index 9cf856c94f94..25bb34c72d20 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c
@@ -97,13 +97,12 @@ static ssize_t amdgpu_securedisplay_debugfs_write(struct 
file *f, const char __u
uint32_t op;
int i;
char str[64];
-   char i2c_output[256];
int ret;
 
if (*pos || size > sizeof(str) - 1)
return -EINVAL;
 
-   memset(str,  0, sizeof(str));
+   memset(str, 0, sizeof(str));
ret = copy_from_user(str, buf, size);
if (ret)
return -EFAULT;
@@ -139,11 +138,9 @@ static ssize_t amdgpu_securedisplay_debugfs_write(struct 
file *f, const char __u
ret = psp_securedisplay_invoke(psp, 
TA_SECUREDISPLAY_COMMAND__SEND_ROI_CRC);
if (!ret) {
if (securedisplay_cmd->status == 
TA_SECUREDISPLAY_STATUS__SUCCESS) {
-   memset(i2c_output,  0, sizeof(i2c_output));
-   for (i = 0; i < 
TA_SECUREDISPLAY_I2C_BUFFER_SIZE; i++)
-   sprintf(i2c_output, "%s 0x%X", 
i2c_output,
-   
securedisplay_cmd->securedisplay_out_message.send_roi_crc.i2c_buf[i]);
-   dev_info(adev->dev, "SECUREDISPLAY: I2C buffer 
out put is :%s\n", i2c_output);
+   dev_info(adev->dev, "SECUREDISPLAY: I2C buffer 
output is: %*ph\n",
+(int)TA_SECUREDISPLAY_I2C_BUFFER_SIZE,
+
securedisplay_cmd->securedisplay_out_message.send_roi_crc.i2c_buf);
} else {
psp_securedisplay_parse_resp_status(psp, 
securedisplay_cmd->status);
}


___
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 Tue, 2020-11-24 at 11:58 +1100, Finn Thain wrote:
> it's not for me to prove that such patches don't affect code 
> generation. That's for the patch author and (unfortunately) for reviewers.

Ideally, that proof would be provided by the compilation system itself
and not patch authors nor reviewers nor maintainers.

Unfortunately gcc does not guarantee repeatability or deterministic output.
To my knowledge, neither does clang.


___
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: [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

[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: [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

[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: [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 000/141] Fix fall-through warnings for Clang

[Joe Perches]

On Fri, 2020-11-20 at 12:21 -0600, Gustavo A. R. Silva wrote:
> Hi all,
> 
> 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.

This was a bit hard to parse for a second or three.

Thanks Gustavo.

How was this change done?


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

Re: [PATCH 01/40] drm/amd/include/vega10_ip_offset: Mark _BASE structs as __maybe_unused

[Joe Perches]

On Fri, 2020-11-13 at 13:48 +, Lee Jones wrote:
> This patch fixes nearly 400 warnings!
> 
> These structures are too widely used in too many varying
> configurations to be split-up into different headers or moved into
> source files.
> 
> Instead, we'll mark them as __maybe_unused which tells the compiler
> that we're aware they're being included into source files which do not
> make use of them - but we've looked into it, and it's okay.

https://gcc.gnu.org/onlinedocs/gcc-4.7.2/gcc/Type-Attributes.html#Type-Attributes

Wouldn't it be simpler to mark the struct definitions as maybe_unused
instead of the declarations?

And perhaps remove all the unnecessary zeroed declarations?

Something like this example?
---
 drivers/gpu/drm/amd/include/arct_ip_offset.h | 353 +++
 1 file changed, 145 insertions(+), 208 deletions(-)

diff --git a/drivers/gpu/drm/amd/include/arct_ip_offset.h 
b/drivers/gpu/drm/amd/include/arct_ip_offset.h
index a7791a9e1f90..9f2d6b832dd9 100644
--- a/drivers/gpu/drm/amd/include/arct_ip_offset.h
+++ b/drivers/gpu/drm/amd/include/arct_ip_offset.h
@@ -33,215 +33,152 @@ struct IP_BASE_INSTANCE
 struct IP_BASE
 {
 struct IP_BASE_INSTANCE instance[MAX_INSTANCE];
-};
-
-
-static const struct IP_BASE ATHUB_BASE={ { { { 0x0C20, 
0x00012460, 0x00408C00, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } } } };
-static const struct IP_BASE CLK_BASE={ { { { 0x000120C0, 
0x00016C00, 0x00401800, 0, 0, 0 } },
-{ { 0x000120E0, 0x00016E00, 
0x00401C00, 0, 0, 0 } },
-{ { 0x00012100, 0x00017000, 
0x00402000, 0, 0, 0 } },
-{ { 0x00012120, 0x00017200, 
0x00402400, 0, 0, 0 } },
-{ { 0x000136C0, 0x0001B000, 
0x0042D800, 0, 0, 0 } },
-{ { 0x00013720, 0x0001B200, 
0x0042E400, 0, 0, 0 } },
-{ { 0x000125E0, 0x00017E00, 
0x0040BC00, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } } } };
-static const struct IP_BASE DF_BASE={ { { { 0x7000, 
0x000125C0, 0x0040B800, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } } } };
-static const struct IP_BASE FUSE_BASE={ { { { 0x000120A0, 
0x00017400, 0x00401400, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } } } };
-static const struct IP_BASE GC_BASE={ { { { 0x2000, 
0xA000, 0x00012160, 0x00402C00, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } } } };
-static const struct IP_BASE HDP_BASE={ { { { 0x0F20, 
0x00012520, 0x0040A400, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } },
-{ { 0, 0, 0, 0, 0, 0 } } } };
-static const struct IP_BASE MMHUB_BASE={ { { { 0x00012440, 
0x0001A000, 0x00408800, 0, 

Re: Subject: [RFC] clang tooling cleanups

[Joe Perches]

On Tue, 2020-10-27 at 09:42 -0700, t...@redhat.com wrote:
> This rfc will describe
> An upcoming treewide cleanup.
> How clang tooling was used to programatically do the clean up.
> Solicit opinions on how to generally use clang tooling.
> 
> The clang warning -Wextra-semi-stmt produces about 10k warnings.
> Reviewing these, a subset of semicolon after a switch looks safe to
> fix all the time.  An example problem
> 
> void foo(int a) {
>  switch(a) {
>  case 1:
>  ...
>  }; <--- extra semicolon
> }
> 
> Treewide, there are about 100 problems in 50 files for x86_64 allyesconfig.
> These fixes will be the upcoming cleanup.

coccinelle already does some of these.

For instance: scripts/coccinelle/misc/semicolon.cocci

Perhaps some tool coordination can be done here as
coccinelle/checkpatch/clang/Lindent call all be used
to do some facet or another of these cleanup issues.



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

Re: [Outreachy kernel] [PATCH] drm/amdgpu: use DEFINE_DEBUGFS_ATTRIBUTE with debugfs_create_file_unsafe()

[Joe Perches]

On Fri, 2020-10-30 at 09:03 +0100, Greg KH wrote:
> On Fri, Oct 30, 2020 at 01:27:16PM +0530, Deepak R Varma wrote:
> > On Fri, Oct 30, 2020 at 08:11:20AM +0100, Greg KH wrote:
> > > On Fri, Oct 30, 2020 at 08:52:45AM +0530, Deepak R Varma wrote:
> > > > Using DEFINE_DEBUGFS_ATTRIBUTE macro with debugfs_create_file_unsafe()
> > > > function in place of the debugfs_create_file() function will make the
> > > > file operation struct "reset" aware of the file's lifetime. Additional
> > > > details here: https://lists.archive.carbon60.com/linux/kernel/2369498
> > > > 
> > > > Issue reported by Coccinelle script:
> > > > scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
[]
> There is a reason we didn't just do a global search/replace for this in
> the kernel when the new functions were added, so I don't know why
> checkpatch is now saying it must be done.

I think it's not a checkpatch warning here.

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

Re: [RFC] treewide: cleanup unreachable breaks

[Joe Perches]

On Mon, 2020-10-19 at 12:42 -0700, Nick Desaulniers wrote:
> On Sat, Oct 17, 2020 at 10:43 PM Greg KH  wrote:
> > On Sat, Oct 17, 2020 at 09:09:28AM -0700, t...@redhat.com wrote:
> > > From: Tom Rix 
> > > 
> > > This is a upcoming change to clean up a new warning treewide.
> > > I am wondering if the change could be one mega patch (see below) or
> > > normal patch per file about 100 patches or somewhere half way by 
> > > collecting
> > > early acks.
> > 
> > Please break it up into one-patch-per-subsystem, like normal, and get it
> > merged that way.
> > 
> > Sending us a patch, without even a diffstat to review, isn't going to
> > get you very far...
> 
> Tom,
> If you're able to automate this cleanup, I suggest checking in a
> script that can be run on a directory.  Then for each subsystem you
> can say in your commit "I ran scripts/fix_whatever.py on this subdir."
>  Then others can help you drive the tree wide cleanup.  Then we can
> enable -Wunreachable-code-break either by default, or W=2 right now
> might be a good idea.
> 
> Ah, George (gbiv@, cc'ed), did an analysis recently of
> `-Wunreachable-code-loop-increment`, `-Wunreachable-code-break`, and
> `-Wunreachable-code-return` for Android userspace.  From the review:
> ```
> Spoilers: of these, it seems useful to turn on
> -Wunreachable-code-loop-increment and -Wunreachable-code-return by
> default for Android
> ...
> While these conventions about always having break arguably became
> obsolete when we enabled -Wfallthrough, my sample turned up zero
> potential bugs caught by this warning, and we'd need to put a lot of
> effort into getting a clean tree. So this warning doesn't seem to be
> worth it.
> ```
> Looks like there's an order of magnitude of `-Wunreachable-code-break`
> than the other two.
> 
> We probably should add all 3 to W=2 builds (wrapped in cc-option).
> I've filed https://github.com/ClangBuiltLinux/linux/issues/1180 to
> follow up on.

I suggest using W=1 as people that are doing cleanups
generally use that and not W=123 or any other style.

Every other use of W= is still quite noisy and these
code warnings are relatively trivially to fix up.


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

Re: [RFC] treewide: cleanup unreachable breaks

[Joe Perches]

On Sat, 2020-10-17 at 09:09 -0700, t...@redhat.com wrote:
> From: Tom Rix 
> 
> This is a upcoming change to clean up a new warning treewide.
> I am wondering if the change could be one mega patch (see below) or
> normal patch per file about 100 patches or somewhere half way by collecting
> early acks.
> 
> clang has a number of useful, new warnings see
> https://clang.llvm.org/docs/DiagnosticsReference.html
> 
> This change cleans up -Wunreachable-code-break
> https://clang.llvm.org/docs/DiagnosticsReference.html#wunreachable-code-break
> for 266 of 485 warnings in this week's linux-next, allyesconfig on x86_64.

Early acks/individual patches by subsystem would be good.
Better still would be an automated cocci script.

The existing checkpatch test for UNNECESSARY_BREAK
has a few too many false positives.

>From a script run on next on July 28th:

arch/arm/mach-s3c24xx/mach-rx1950.c:266: WARNING:UNNECESSARY_BREAK: break is 
not useful after a goto or return
arch/arm/nwfpe/fpa11_cprt.c:38: WARNING:UNNECESSARY_BREAK: break is not useful 
after a goto or return
arch/arm/nwfpe/fpa11_cprt.c:41: WARNING:UNNECESSARY_BREAK: break is not useful 
after a goto or return
arch/mips/include/asm/mach-au1x00/au1000.h:684: WARNING:UNNECESSARY_BREAK: 
break is not useful after a goto or return
arch/mips/include/asm/mach-au1x00/au1000.h:687: WARNING:UNNECESSARY_BREAK: 
break is not useful after a goto or return
arch/mips/include/asm/mach-au1x00/au1000.h:690: WARNING:UNNECESSARY_BREAK: 
break is not useful after a goto or return
arch/mips/include/asm/mach-au1x00/au1000.h:693: WARNING:UNNECESSARY_BREAK: 
break is not useful after a goto or return
arch/mips/include/asm/mach-au1x00/au1000.h:697: WARNING:UNNECESSARY_BREAK: 
break is not useful after a goto or return
arch/mips/include/asm/mach-au1x00/au1000.h:700: WARNING:UNNECESSARY_BREAK: 
break is not useful after a goto or return
arch/mips/loongson2ef/common/cs5536/cs5536_isa.c:276: 
WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return
arch/mips/loongson2ef/common/cs5536/cs5536_isa.c:279: 
WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return
arch/mips/loongson2ef/common/cs5536/cs5536_isa.c:282: 
WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return
arch/mips/loongson2ef/common/cs5536/cs5536_isa.c:287: 
WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return
arch/mips/loongson2ef/common/cs5536/cs5536_isa.c:290: 
WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return
arch/mips/rb532/setup.c:76: WARNING:UNNECESSARY_BREAK: break is not useful 
after a goto or return
arch/mips/rb532/setup.c:79: WARNING:UNNECESSARY_BREAK: break is not useful 
after a goto or return
arch/powerpc/include/asm/kvm_book3s_64.h:231: WARNING:UNNECESSARY_BREAK: break 
is not useful after a goto or return
arch/powerpc/include/asm/kvm_book3s_64.h:234: WARNING:UNNECESSARY_BREAK: break 
is not useful after a goto or return
arch/powerpc/include/asm/kvm_book3s_64.h:237: WARNING:UNNECESSARY_BREAK: break 
is not useful after a goto or return
arch/powerpc/include/asm/kvm_book3s_64.h:240: WARNING:UNNECESSARY_BREAK: break 
is not useful after a goto or return
arch/powerpc/net/bpf_jit_comp.c:455: WARNING:UNNECESSARY_BREAK: break is not 
useful after a goto or return
arch/powerpc/platforms/cell/spufs/switch.c:2047: WARNING:UNNECESSARY_BREAK: 
break is not useful after a goto or return
arch/powerpc/platforms/cell/spufs/switch.c:2077: WARNING:UNNECESSARY_BREAK: 
break is not useful after a goto or return
arch/sh/boards/mach-landisk/gio.c:111: WARNING:UNNECESSARY_BREAK: break is not 
useful after a goto or return
arch/x86/kernel/cpu/mce/core.c:1734: WARNING:UNNECESSARY_BREAK: break is not 
useful after a goto or return
arch/x86/kernel/cpu/mce/core.c:1738: WARNING:UNNECESSARY_BREAK: break is not 
useful after a goto or return
arch/x86/kernel/cpu/microcode/amd.c:218: WARNING:UNNECESSARY_BREAK: break is 
not useful after a goto or return
drivers/acpi/utils.c:107: WARNING:UNNECESSARY_BREAK: break is not useful after 
a goto or return
drivers/acpi/utils.c:132: WARNING:UNNECESSARY_BREAK: break is not useful after 
a goto or return
drivers/acpi/utils.c:147: WARNING:UNNECESSARY_BREAK: break is not useful after 
a goto or return
drivers/acpi/utils.c:158: WARNING:UNNECESSARY_BREAK: break is not useful after 
a goto or return
drivers/ata/libata-scsi.c:3973: WARNING:UNNECESSARY_BREAK: break is not useful 
after a goto or return
drivers/base/power/main.c:366: WARNING:UNNECESSARY_BREAK: break is not useful 
after a goto or return
drivers/block/xen-blkback/blkback.c:1272: WARNING:UNNECESSARY_BREAK: break is 
not useful after a goto or return
drivers/char/ipmi/ipmi_devintf.c:493: WARNING:UNNECESSARY_BREAK: break is not 
useful after a goto or return
drivers/char/lp.c:625: WARNING:UNNECESSARY_BREAK: break is not useful after a 
goto or return
drivers/char/mwave/mwavedd.c:406: WARNING:UNNECESSARY_BREAK: break is not 
useful after a goto or return

Re: [Cocci] [RFC] treewide: cleanup unreachable breaks

[Joe Perches]

On Sat, 2020-10-17 at 20:21 +0200, Julia Lawall wrote:
> On Sat, 17 Oct 2020, Joe Perches wrote:
> > On Sat, 2020-10-17 at 09:09 -0700, t...@redhat.com wrote:
> > > From: Tom Rix 
> > > 
> > > This is a upcoming change to clean up a new warning treewide.
> > > I am wondering if the change could be one mega patch (see below) or
> > > normal patch per file about 100 patches or somewhere half way by 
> > > collecting
> > > early acks.
> > > 
> > > clang has a number of useful, new warnings see
> > > https://clang.llvm.org/docs/DiagnosticsReference.html
> > > 
> > > This change cleans up -Wunreachable-code-break
> > > https://clang.llvm.org/docs/DiagnosticsReference.html#wunreachable-code-break
> > > for 266 of 485 warnings in this week's linux-next, allyesconfig on x86_64.
> > 
> > Early acks/individual patches by subsystem would be good.
> > Better still would be an automated cocci script.
> 
> Coccinelle is not especially good at this, because it is based on control
> flow, and a return or goto diverts the control flow away from the break.
> A hack to solve the problem is to put an if around the return or goto, but
> that gives the break a meaningless file name and line number.  I collected
> the following list, but it only has 439 results, so fewer than clang.  But
> maybe there are some files that are not considered by clang in the x86
> allyesconfig configuration.
> 
> Probably checkpatch is the best solution here, since it is not
> configuration sensitive and doesn't care about control flow.

Likely the clang compiler is the best option here.

It might be useful to add -Wunreachable-code-break to W=1
or just always enable it if it isn't already enabled.

diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 95e4cdb94fe9..3819787579d5 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -32,6 +32,7 @@ KBUILD_CFLAGS += $(call cc-option, -Wunused-but-set-variable)
 KBUILD_CFLAGS += $(call cc-option, -Wunused-const-variable)
 KBUILD_CFLAGS += $(call cc-option, -Wpacked-not-aligned)
 KBUILD_CFLAGS += $(call cc-option, -Wstringop-truncation)
+KBUILD_CFLAGS += $(call cc-option, -Wunreachable-code-break)
 # The following turn off the warnings enabled by -Wextra
 KBUILD_CFLAGS += -Wno-missing-field-initializers
 KBUILD_CFLAGS += -Wno-sign-compare

(and thank you Tom for pushing this forward)

checkpatch can't find instances like:

case FOO:
if (foo)
return 1;
else
return 2;
break;

As it doesn't track flow and relies on the number
of tabs to be the same for any goto/return and break;

checkpatch will warn on:

case FOO:
...
goto bar;
break;


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

Re: [Ocfs2-devel] [RFC] treewide: cleanup unreachable breaks

[Joe Perches]

On Sun, 2020-10-18 at 19:59 +0100, Matthew Wilcox wrote:
> On Sat, Oct 17, 2020 at 09:09:28AM -0700, t...@redhat.com wrote:
> > clang has a number of useful, new warnings see
> > https://urldefense.com/v3/__https://clang.llvm.org/docs/DiagnosticsReference.html__;!!GqivPVa7Brio!Krxz78O3RKcB9JBMVo_F98FupVhj_jxX60ddN6tKGEbv_cnooXc1nnBmchm-e_O9ieGnyQ$
> >  
> 
> Please get your IT department to remove that stupidity.  If you can't,
> please send email from a non-Red Hat email address.

I didn't get it this way, neither did lore.
It's on your end.

https://lore.kernel.org/lkml/20201017160928.12698-1-t...@redhat.com/

> I don't understand why this is a useful warning to fix.

Precision in coding style intent and code minimization
would be the biggest factors IMO.

> What actual problem is caused by the code below?

Obviously none.


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

Re: [RFC PATCH] DRM: amd: powerplay: don't undef pr_warn() {causes ARC build errors}

[Joe Perches]

On Mon, 2020-10-05 at 21:50 -0700, Randy Dunlap wrote:
> From: Randy Dunlap 
> 
> arch/arc/ implements BUG_ON() with BUG(). ARC has its own BUG()
> function and that function uses pr_warn() as part of its implementation.
> 
> Several (8) files in amd/powerplay/ #undef various pr_xyz() functions so
> that they won't be used by these drivers, since dev_() functions are
> preferred here and the #undefs make the pr_() functions unavailable.
[]
> --- lnx-59-rc7.orig/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> +++ lnx-59-rc7/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> @@ -52,7 +52,7 @@
>   * They are more MGPU friendly.
>   */
>  #undef pr_err
> -#undef pr_warn
> +//#undef pr_warn
>  #undef pr_info
>  #undef pr_debug
>  
> --- lnx-59-rc7.orig/drivers/gpu/drm/amd/powerplay/sienna_cichlid_ppt.c
> +++ lnx-59-rc7/drivers/gpu/drm/amd/powerplay/sienna_cichlid_ppt.c
> @@ -54,7 +54,7 @@
>   * They are more MGPU friendly.
>   */
>  #undef pr_err
> -#undef pr_warn
> +//#undef pr_warn
>  #undef pr_info
>  #undef pr_debug 

These are bad ideas as all of these pr_ entries
may become functions in a near-term future.


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

Re: [PATCH] drm/amdgpu: fix spelling mistake "Falied" -> "Failed"

[Joe Perches]

On Wed, 2020-08-05 at 17:27 -0400, Alex Deucher wrote:
> On Wed, Aug 5, 2020 at 4:53 PM Joe Perches  wrote:
> > On Wed, 2020-08-05 at 16:01 -0400, Alex Deucher wrote:
> > > On Wed, Aug 5, 2020 at 7:35 AM Colin King  
> > > wrote:
> > > > From: Colin Ian King 
> > > > 
> > > > There is a spelling mistake in a DRM_ERROR message. Fix it.
> > > > 
> > > > Signed-off-by: Colin Ian King 
> > > 
> > > This is already fixed.
> > 
> > This fix is not in today's -next.
> > 
> > Perhaps whatever tree it's fixed in should be in -next.
> > 
> 
> Weird.  It's in the drm-next tree as:
> 
> commit 4afaa61db9cf5250b5734c2531b226e7b3a3d691
> Author: Colin Ian King 
> Date:   Fri Jul 10 09:37:58 2020 +0100
> 
> drm/amdgpu: fix spelling mistake "Falied" -> "Failed"
> 
> There is a spelling mistake in a DRM_ERROR error message. Fix it.
> 
> Signed-off-by: Colin Ian King 
> Signed-off-by: Alex Deucher 
> 
> Alex
> 
> > $ git show --oneline -s
> > d15fe4ec0435 (HEAD, tag: next-20200805, origin/master, origin/HEAD) Add 
> > linux-next specific files for 20200805
> > 
> > $ git grep -i falied drivers
> > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c:DRM_ERROR("Falied 
> > to terminate tmr\n");
> > 
> > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
> > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > []
> > > > @@ -2010,7 +2010,7 @@ static int psp_suspend(void *handle)
> > > > 
> > > > ret = psp_tmr_terminate(psp);
> > > > if (ret) {
> > > > -   DRM_ERROR("Falied to terminate tmr\n");
> > > > +   DRM_ERROR("Failed to terminate tmr\n");
> > > > return ret;
> > > > }

Dunno.

Maybe it's due to some ordering of trees in
how -next accumulates patches?


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

Re: [PATCH] drm/amdgpu: fix spelling mistake "Falied" -> "Failed"

[Joe Perches]

On Wed, 2020-08-05 at 16:01 -0400, Alex Deucher wrote:
> On Wed, Aug 5, 2020 at 7:35 AM Colin King  wrote:
> > From: Colin Ian King 
> > 
> > There is a spelling mistake in a DRM_ERROR message. Fix it.
> > 
> > Signed-off-by: Colin Ian King 
> 
> This is already fixed.

This fix is not in today's -next.

Perhaps whatever tree it's fixed in should be in -next.


$ git show --oneline -s
d15fe4ec0435 (HEAD, tag: next-20200805, origin/master, origin/HEAD) Add 
linux-next specific files for 20200805

$ git grep -i falied drivers
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c:DRM_ERROR("Falied to 
terminate tmr\n");

> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
[]
> > @@ -2010,7 +2010,7 @@ static int psp_suspend(void *handle)
> > 
> > ret = psp_tmr_terminate(psp);
> > if (ret) {
> > -   DRM_ERROR("Falied to terminate tmr\n");
> > +   DRM_ERROR("Failed to terminate tmr\n");
> > return ret;
> > }


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

Re: [PATCH] drm/amd/display: Fix indenting in dcn30_set_output_transfer_func()

[Joe Perches]

On Mon, 2020-06-08 at 20:49 +0300, Dan Carpenter wrote:
> On Mon, Jun 08, 2020 at 10:16:27AM -0700, Joe Perches wrote:
> > On Mon, 2020-06-08 at 17:16 +0300, Dan Carpenter wrote:
> > > These lines are a part of the if statement and they are supposed to
> > > be indented one more tab.
> > > 
> > > Signed-off-by: Dan Carpenter 
> > > ---
> > >  drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c 
> > > b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> > > index ab20320ebc994..37c310dbb3665 100644
> > > --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> > > +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> > > @@ -203,9 +203,9 @@ bool dcn30_set_output_transfer_func(struct dc *dc,
> > >   stream->out_transfer_func,
> > >   >blender_params, false))
> > >   params = >blender_params;
> > > -  /* there are no ROM LUTs in OUTGAM */
> > > - if (stream->out_transfer_func->type == TF_TYPE_PREDEFINED)
> > > - BREAK_TO_DEBUGGER();
> > > +  /* there are no ROM LUTs in OUTGAM */
> > > + if (stream->out_transfer_func->type == 
> > > TF_TYPE_PREDEFINED)
> > > + BREAK_TO_DEBUGGER();
> > >   }
> > >   }
> > >  
> > 
> > Maybe the if is at the right indentation but the
> > close brace below the if is misplaced instead?
> > 
> 
> Yeah.  I considered that, but the code is correct, it's just the
> indenting is wrong.  I normally leave drm/amd/ code alone but this
> indenting was so confusing that I though it was worth fixing.

This file seems to heavily use function pointers,
multiple dereferences
with visually similar identifiers,
and it generally makes my eyes hurt
reading the code.

> There are lots of ugly stuff which is not confusing like this:  (The
> line numbers are from next-20200605).

Ick.  Don't give me line numbers.  Now I might have to look...

drivers/gpu/drm/amd/amdgpu/../powerplay/amd_powerplay.c:1530 
pp_asic_reset_mode_2() warn: inconsistent indenting
> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:3387 bw_calcs() 
> warn: inconsistent indenting
> drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_dwb.c:104 dwb2_enable() 
> warn: inconsistent indenting
> drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_dpp_cm.c:450 
> dpp20_get_blndgam_current() warn: inconsistent indenting
> drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_dpp_cm.c:543 
> dpp20_get_shaper_current() warn: inconsistent indenting
> drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_mpc.c:306 
> mpc20_get_ogam_current() warn: inconsistent indenting
> drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:1519 
> dc_link_dp_perform_link_training() warn: inconsistent indenting
> drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:3137 
> core_link_enable_stream() warn: inconsistent indenting
> drivers/gpu/drm/amd/amdgpu/../display/dc/dcn30/dcn30_hwseq.c:207 
> dcn30_set_output_transfer_func() warn: inconsistent indenting
> drivers/gpu/drm/amd/amdgpu/../display/dc/dcn30/dcn30_dpp.c:650 
> dpp3_get_blndgam_current() warn: inconsistent indenting

OK, so I picked this one at random.

It looks like someone avoided using intentional programming
along with copy/paste combined with being lazy.

It seems as if AMD should use more code reviewers and
perhaps some automated code reformatters before submitting
their code.

This code is:

static enum dc_lut_mode dpp3_get_blndgam_current(struct dpp *dpp_base)
{
enum dc_lut_mode mode;
uint32_t mode_current = 0;
uint32_t in_use = 0;

struct dcn3_dpp *dpp = TO_DCN30_DPP(dpp_base);

REG_GET(CM_BLNDGAM_CONTROL,
CM_BLNDGAM_MODE_CURRENT, _current);
REG_GET(CM_BLNDGAM_CONTROL,
CM_BLNDGAM_SELECT_CURRENT, _use);

switch (mode_current) {
case 0:
case 1:
mode = LUT_BYPASS;
break;

case 2:
if (in_use == 0)
mode = LUT_RAM_A;
else
mode = LUT_RAM_B;
break;
default:
mode = LUT_BYPASS;
break;
}
return mode;
}

Generic style defects:

o unnecessary initialization

Re: [PATCH] drm/amd/display: Fix indenting in dcn30_set_output_transfer_func()

[Joe Perches]

On Mon, 2020-06-08 at 17:16 +0300, Dan Carpenter wrote:
> These lines are a part of the if statement and they are supposed to
> be indented one more tab.
> 
> Signed-off-by: Dan Carpenter 
> ---
>  drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c 
> b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> index ab20320ebc994..37c310dbb3665 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> @@ -203,9 +203,9 @@ bool dcn30_set_output_transfer_func(struct dc *dc,
>   stream->out_transfer_func,
>   >blender_params, false))
>   params = >blender_params;
> -  /* there are no ROM LUTs in OUTGAM */
> - if (stream->out_transfer_func->type == TF_TYPE_PREDEFINED)
> - BREAK_TO_DEBUGGER();
> +  /* there are no ROM LUTs in OUTGAM */
> + if (stream->out_transfer_func->type == 
> TF_TYPE_PREDEFINED)
> + BREAK_TO_DEBUGGER();
>   }
>   }
>  

Maybe the if is at the right indentation but the
close brace below the if is misplaced instead?

Also, because this code uses very long identifiers,
it would read better using wider columns as the
logic in the code itself isn't complicated but the
80 column wrapping makes it seem so.

Wrapping could be something like:
---
diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c 
b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
index ab20320ebc99..56e91a73610f 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
@@ -190,18 +190,16 @@ bool dcn30_set_output_transfer_func(struct dc *dc,
struct pwl_params *params = NULL;
bool ret = false;
 
-   /* program OGAM or 3DLUT only for the top pipe*/
+   /* program OGAM or 3DLUT only for the top pipe */
if (pipe_ctx->top_pipe == NULL) {
-   /*program rmu shaper and 3dlut in MPC*/
+   /* program rmu shaper and 3dlut in MPC */
ret = dcn30_set_mpc_shaper_3dlut(pipe_ctx, stream);
-   if (ret == false && mpc->funcs->set_output_gamma && 
stream->out_transfer_func) {
+   if (!ret && mpc->funcs->set_output_gamma && 
stream->out_transfer_func) {
if (stream->out_transfer_func->type == TF_TYPE_HWPWL)
params = >out_transfer_func->pwl;
-   else if (pipe_ctx->stream->out_transfer_func->type ==
-   TF_TYPE_DISTRIBUTED_POINTS &&
-   cm3_helper_translate_curve_to_hw_format(
-   stream->out_transfer_func,
-   >blender_params, false))
+   else if (pipe_ctx->stream->out_transfer_func->type == 
TF_TYPE_DISTRIBUTED_POINTS &&
+
cm3_helper_translate_curve_to_hw_format(stream->out_transfer_func,
+
>blender_params, false))
params = >blender_params;
 /* there are no ROM LUTs in OUTGAM */
if (stream->out_transfer_func->type == TF_TYPE_PREDEFINED)


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

Re: [PATCH] drm/amdgpu: allocate large structures dynamically

[Joe Perches]

On Thu, 2020-05-07 at 08:42 +0200, Christian König wrote:
> Am 06.05.20 um 21:01 schrieb Joe Perches:
[]
> > And trivia:
> > 
> > The !! uses with bool seem unnecessary and it's probably better
> > to make amdgpu_ras_is_feature_enabled to return bool.
[]
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
[]
> > @@ -560,7 +560,7 @@ static int __amdgpu_ras_feature_enable(struct 
> > amdgpu_device *adev,
> >  */
> > if (!amdgpu_ras_is_feature_allowed(adev, head))
> > return 0;
> > -   if (!(!!enable ^ !!amdgpu_ras_is_feature_enabled(adev, head)))
> > +   if (!(enable ^ amdgpu_ras_is_feature_enabled(adev, head)))
> 
> And while we are at improving coding style I think that writing this as 
> "if (enabled == amdgpu_ras_is_feature_enabled(adev, head))" would be 
> much more readable.

 that's decidedly true...


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

Re: [PATCH] drm/amdgpu: allocate large structures dynamically

[Joe Perches]

On Tue, 2020-05-05 at 16:01 +0200, Arnd Bergmann wrote:
> After the structure was padded to 1024 bytes, it is no longer
> suitable for being a local variable, as the function surpasses
> the warning limit for 32-bit architectures:
> 
> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:587:5: error: stack frame size of 
> 1072 bytes in function 'amdgpu_ras_feature_enable' 
> [-Werror,-Wframe-larger-than=]
> int amdgpu_ras_feature_enable(struct amdgpu_device *adev,
> ^
> 
> Use kzalloc() instead to get it from the heap.
[]
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
[]
> @@ -588,19 +588,23 @@ int amdgpu_ras_feature_enable(struct amdgpu_device 
> *adev,
>   struct ras_common_if *head, bool enable)
>  {
>   struct amdgpu_ras *con = amdgpu_ras_get_context(adev);
> - union ta_ras_cmd_input info;
> + union ta_ras_cmd_input *info;
>   int ret;
>  
>   if (!con)
>   return -EINVAL;
>  
> +info = kzalloc(sizeof(union ta_ras_cmd_input), GFP_KERNEL);

Spaces were used for indentation here not a tab.
It might be useful to run your proposed patches through checkpatch

Is this an actual bug fix as the previous use didn't
zero unused info members?

> + if (!info)
> + return -ENOMEM;
> +
>   if (!enable) {
> - info.disable_features = (struct ta_ras_disable_features_input) {
> + info->disable_features = (struct ta_ras_disable_features_input) 
> {
>   .block_id =  amdgpu_ras_block_to_ta(head->block),
>   .error_type = amdgpu_ras_error_to_ta(head->type),
>   };
>   } else {
> - info.enable_features = (struct ta_ras_enable_features_input) {
> + info->enable_features = (struct ta_ras_enable_features_input) {
>   .block_id =  amdgpu_ras_block_to_ta(head->block),
>   .error_type = amdgpu_ras_error_to_ta(head->type),
>   };
> @@ -609,26 +613,33 @@ int amdgpu_ras_feature_enable(struct amdgpu_device 
> *adev,
>   /* Do not enable if it is not allowed. */
>   WARN_ON(enable && !amdgpu_ras_is_feature_allowed(adev, head));
>   /* Are we alerady in that state we are going to set? */
> - if (!(!!enable ^ !!amdgpu_ras_is_feature_enabled(adev, head)))
> - return 0;
> + if (!(!!enable ^ !!amdgpu_ras_is_feature_enabled(adev, head))) {

And trivia:

The !! uses with bool seem unnecessary and it's probably better
to make amdgpu_ras_is_feature_enabled to return bool.

Maybe something like:
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index 538895cfd862..05c4eaf0ddfa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -526,16 +526,16 @@ void amdgpu_ras_parse_status_code(struct amdgpu_device* 
adev,
 }
 
 /* feature ctl begin */
-static int amdgpu_ras_is_feature_allowed(struct amdgpu_device *adev,
-   struct ras_common_if *head)
+static bool amdgpu_ras_is_feature_allowed(struct amdgpu_device *adev,
+ struct ras_common_if *head)
 {
struct amdgpu_ras *con = amdgpu_ras_get_context(adev);
 
return con->hw_supported & BIT(head->block);
 }
 
-static int amdgpu_ras_is_feature_enabled(struct amdgpu_device *adev,
-   struct ras_common_if *head)
+static bool amdgpu_ras_is_feature_enabled(struct amdgpu_device *adev,
+ struct ras_common_if *head)
 {
struct amdgpu_ras *con = amdgpu_ras_get_context(adev);
 
@@ -560,7 +560,7 @@ static int __amdgpu_ras_feature_enable(struct amdgpu_device 
*adev,
 */
if (!amdgpu_ras_is_feature_allowed(adev, head))
return 0;
-   if (!(!!enable ^ !!amdgpu_ras_is_feature_enabled(adev, head)))
+   if (!(enable ^ amdgpu_ras_is_feature_enabled(adev, head)))
return 0;
 
if (enable) {
@@ -609,7 +609,7 @@ int amdgpu_ras_feature_enable(struct amdgpu_device *adev,
/* Do not enable if it is not allowed. */
WARN_ON(enable && !amdgpu_ras_is_feature_allowed(adev, head));
/* Are we alerady in that state we are going to set? */
-   if (!(!!enable ^ !!amdgpu_ras_is_feature_enabled(adev, head)))
+   if (!(enable ^ amdgpu_ras_is_feature_enabled(adev, head)))
return 0;
 
if (!amdgpu_ras_intr_triggered()) {



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

Re: [PATCH] drm/radeon: cleanup coding style a bit

[Joe Perches]

On Sun, 2020-04-26 at 15:18 +0200, Christian König wrote:
> Am 26.04.20 um 15:12 schrieb Bernard Zhao:
> > Maybe no need to check ws before kmalloc, kmalloc will check
> > itself, kmalloc`s logic is if ptr is NULL, kmalloc will just
> > return
> > 
> > Signed-off-by: Bernard Zhao 
> 
> Reviewed-by: Christian König 
> 
> I'm wondering why the automated scripts haven't found that one before.

because this pattern is

if (foo)
kfree(bar);

and the pattern looked for is:

if (foo)
kfree(foo);

> > diff --git a/drivers/gpu/drm/radeon/atom.c b/drivers/gpu/drm/radeon/atom.c
[]
> > @@ -1211,8 +1211,7 @@ static int atom_execute_table_locked(struct 
> > atom_context *ctx, int index, uint32
> > SDEBUG("<<\n");
> >   
> >   free:
> > -   if (ws)
> > -   kfree(ectx.ws);
> > +   kfree(ectx.ws);
> > return ret;
> >   }

I'm wondering if this removal is correct as the function
is named _locked and it may be recursive or called under
some external lock.


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

Re: [PATCH] drm/radeon: cleanup coding style a bit

[Joe Perches]

btw:  the debugging macros in atom.c are not good.

It could be something like the below as the output logging is
at best poorly formatted due to the many individual printks
without KERN_ that are emitted on separate lines.

#define ATOM_DEBUG

should probably be commented out.

The debugging macros and #include file should be better formatted.

The no_printk macro is useful to verify formats and arguments when
not debugging and removing the ATOM_DEBUG from atom-names.h does
not cause the unused char *arrays to be added to the object file
as the compiler elides unused arrays.

---
 drivers/gpu/drm/radeon/atom-names.h | 266 +++-
 drivers/gpu/drm/radeon/atom.c   |  23 ++--
 2 files changed, 218 insertions(+), 71 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atom-names.h 
b/drivers/gpu/drm/radeon/atom-names.h
index 6f907a5..055775 100644
--- a/drivers/gpu/drm/radeon/atom-names.h
+++ b/drivers/gpu/drm/radeon/atom-names.h
@@ -27,74 +27,218 @@
 
 #include "atom.h"
 
-#ifdef ATOM_DEBUG
-
 #define ATOM_OP_NAMES_CNT 123
-static char *atom_op_names[ATOM_OP_NAMES_CNT] = {
-"RESERVED", "MOVE_REG", "MOVE_PS", "MOVE_WS", "MOVE_FB", "MOVE_PLL",
-"MOVE_MC", "AND_REG", "AND_PS", "AND_WS", "AND_FB", "AND_PLL", "AND_MC",
-"OR_REG", "OR_PS", "OR_WS", "OR_FB", "OR_PLL", "OR_MC", "SHIFT_LEFT_REG",
-"SHIFT_LEFT_PS", "SHIFT_LEFT_WS", "SHIFT_LEFT_FB", "SHIFT_LEFT_PLL",
-"SHIFT_LEFT_MC", "SHIFT_RIGHT_REG", "SHIFT_RIGHT_PS", "SHIFT_RIGHT_WS",
-"SHIFT_RIGHT_FB", "SHIFT_RIGHT_PLL", "SHIFT_RIGHT_MC", "MUL_REG",
-"MUL_PS", "MUL_WS", "MUL_FB", "MUL_PLL", "MUL_MC", "DIV_REG", "DIV_PS",
-"DIV_WS", "DIV_FB", "DIV_PLL", "DIV_MC", "ADD_REG", "ADD_PS", "ADD_WS",
-"ADD_FB", "ADD_PLL", "ADD_MC", "SUB_REG", "SUB_PS", "SUB_WS", "SUB_FB",
-"SUB_PLL", "SUB_MC", "SET_ATI_PORT", "SET_PCI_PORT", "SET_SYS_IO_PORT",
-"SET_REG_BLOCK", "SET_FB_BASE", "COMPARE_REG", "COMPARE_PS",
-"COMPARE_WS", "COMPARE_FB", "COMPARE_PLL", "COMPARE_MC", "SWITCH",
-"JUMP", "JUMP_EQUAL", "JUMP_BELOW", "JUMP_ABOVE", "JUMP_BELOW_OR_EQUAL",
-"JUMP_ABOVE_OR_EQUAL", "JUMP_NOT_EQUAL", "TEST_REG", "TEST_PS", "TEST_WS",
-"TEST_FB", "TEST_PLL", "TEST_MC", "DELAY_MILLISEC", "DELAY_MICROSEC",
-"CALL_TABLE", "REPEAT", "CLEAR_REG", "CLEAR_PS", "CLEAR_WS", "CLEAR_FB",
-"CLEAR_PLL", "CLEAR_MC", "NOP", "EOT", "MASK_REG", "MASK_PS", "MASK_WS",
-"MASK_FB", "MASK_PLL", "MASK_MC", "POST_CARD", "BEEP", "SAVE_REG",
-"RESTORE_REG", "SET_DATA_BLOCK", "XOR_REG", "XOR_PS", "XOR_WS", "XOR_FB",
-"XOR_PLL", "XOR_MC", "SHL_REG", "SHL_PS", "SHL_WS", "SHL_FB", "SHL_PLL",
-"SHL_MC", "SHR_REG", "SHR_PS", "SHR_WS", "SHR_FB", "SHR_PLL", "SHR_MC",
-"DEBUG", "CTB_DS",
+static const char * const atom_op_names[ATOM_OP_NAMES_CNT] = {
+   "RESERVED",
+   "MOVE_REG",
+   "MOVE_PS",
+   "MOVE_WS",
+   "MOVE_FB",
+   "MOVE_PLL",
+   "MOVE_MC",
+   "AND_REG",
+   "AND_PS",
+   "AND_WS",
+   "AND_FB",
+   "AND_PLL",
+   "AND_MC",
+   "OR_REG",
+   "OR_PS",
+   "OR_WS",
+   "OR_FB",
+   "OR_PLL",
+   "OR_MC",
+   "SHIFT_LEFT_REG",
+   "SHIFT_LEFT_PS",
+   "SHIFT_LEFT_WS",
+   "SHIFT_LEFT_FB",
+   "SHIFT_LEFT_PLL",
+   "SHIFT_LEFT_MC",
+   "SHIFT_RIGHT_REG",
+   "SHIFT_RIGHT_PS",
+   "SHIFT_RIGHT_WS",
+   "SHIFT_RIGHT_FB",
+   "SHIFT_RIGHT_PLL",
+   "SHIFT_RIGHT_MC",
+   "MUL_REG",
+   "MUL_PS",
+   "MUL_WS",
+   "MUL_FB",
+   "MUL_PLL",
+   "MUL_MC",
+   "DIV_REG",
+   "DIV_PS",
+   "DIV_WS",
+   "DIV_FB",
+   "DIV_PLL",
+   "DIV_MC",
+   "ADD_REG",
+   "ADD_PS",
+   "ADD_WS",
+   "ADD_FB",
+   "ADD_PLL",
+   "ADD_MC",
+   "SUB_REG",
+   "SUB_PS",
+   "SUB_WS",
+   "SUB_FB",
+   "SUB_PLL",
+   "SUB_MC",
+   "SET_ATI_PORT",
+   "SET_PCI_PORT",
+   "SET_SYS_IO_PORT",
+   "SET_REG_BLOCK",
+   "SET_FB_BASE",
+   "COMPARE_REG",
+   "COMPARE_PS",
+   "COMPARE_WS",
+   "COMPARE_FB",
+   "COMPARE_PLL",
+   "COMPARE_MC",
+   "SWITCH",
+   "JUMP",
+   "JUMP_EQUAL",
+   "JUMP_BELOW",
+   "JUMP_ABOVE",
+   "JUMP_BELOW_OR_EQUAL",
+   "JUMP_ABOVE_OR_EQUAL",
+   "JUMP_NOT_EQUAL",
+   "TEST_REG",
+   "TEST_PS",
+   "TEST_WS",
+   "TEST_FB",
+   "TEST_PLL",
+   "TEST_MC",
+   "DELAY_MILLISEC",
+   "DELAY_MICROSEC",
+   "CALL_TABLE",
+   "REPEAT",
+   "CLEAR_REG",
+   "CLEAR_PS",
+   "CLEAR_WS",
+   "CLEAR_FB",
+   "CLEAR_PLL",
+   "CLEAR_MC",
+   "NOP",
+   "EOT",
+   "MASK_REG",
+   "MASK_PS",
+   "MASK_WS",
+   "MASK_FB",
+   "MASK_PLL",
+   "MASK_MC",
+   "POST_CARD",
+   "BEEP",
+   "SAVE_REG",
+   "RESTORE_REG",
+   "SET_DATA_BLOCK",
+   "XOR_REG",
+   "XOR_PS",
+   "XOR_WS",
+   "XOR_FB",
+   "XOR_PLL",
+   "XOR_MC",
+   "SHL_REG",
+   "SHL_PS",
+   

Re: [PATCH] drm/amd/display: remove conversion to bool in dcn20_mpc.c

[Joe Perches]

On Mon, 2020-04-27 at 14:37 +0800, Jason Yan wrote:
> The '==' expression itself is bool, no need to convert it to bool again.

trivia:

These descriptions are not quite correct.
The operators return an int, either 0 or 1.

-
6.5.8 Relational operators

6 Each of the operators < (less than), > (greater than), <= (less than or equal 
to), and >=
(greater than or equal to) shall yield 1 if the specified relation is true and 
0 if it is false. 90)
The result has type int

6.5.9 Equality operators

3 The == (equal to) and != (not equal to) operators are analogous to the 
relational
operators except for their lower precedence. 91) Each of the operators yields 1 
if the
specified relation is true and 0 if it is false. The result has type int. For 
any pair of
operands, exactly one of the relations is true.
-




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

Re: [PATCH] drm/amd/amdgpu: add prefix for pr_* prints

[Joe Perches]

On Wed, 2020-04-08 at 09:37 -0400, Aurabindo Pillai wrote:
> amdgpu uses lots of pr_* calls for printing error messages.
> With this prefix, errors shall be more obvious to the end
> use regarding its origin, and may help debugging.
> 
> Prefix format:
> 
> [xxx.x] amdgpu: ...
[]
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
[]
> @@ -28,6 +28,12 @@
>  #ifndef __AMDGPU_H__
>  #define __AMDGPU_H__
>  
> +#ifdef pr_fmt
> +#undef pr_fmt
> +#endif
> +
> +#define pr_fmt(fmt) "amdgpu: " fmt
> +
>  #include "amdgpu_ctx.h"
>  
>  #include 

All the embedded uses of "amdgpu:" in logging
messages should also be deleted.

$ git grep -P '(?:dev_|pr_).*"amdgpu:' drivers/gpu/drm/amd/amdgpu/
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c:   pr_err("amdgpu: 
failed to validate PT BOs\n");
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c:   pr_err("amdgpu: 
failed to validate PD\n");
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c:   
pr_err("amdgpu: failed to kmap PD, ret=%d\n", ret);
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c: pr_info("amdgpu: 
switched on\n");
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c: pr_info("amdgpu: 
switched off\n");
drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c:
dev_dbg(adev->dev, "amdgpu: using MSI/MSI-X.\n");
drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c:  dev_warn(adev->dev, "amdgpu: No 
suitable DMA available.\n");
drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c:  pr_warn("amdgpu: No suitable 
DMA available\n");
drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c:  pr_warn("amdgpu: No suitable 
DMA available\n");



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

Re: [PATCH] drm/amdgpu: Initialize shadow to false in gfx_v9_0_rlcg_wreg

[Joe Perches]

On Tue, 2020-03-17 at 17:25 -0700, Nathan Chancellor wrote:
> clang warns:
> 
> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:754:6: warning: variable 'shadow'
> is used uninitialized whenever 'if' condition is
> false [-Wsometimes-uninitialized]
> if (offset == grbm_cntl || offset == grbm_idx)
> ^
> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:757:6: note: uninitialized use
> occurs here
> if (shadow) {
> ^~

Wouldn't it be better to get rid of the shadow variable completely?
---
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 7bc248..496b9e 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -735,7 +735,6 @@ void gfx_v9_0_rlcg_wreg(struct amdgpu_device *adev, u32 
offset, u32 v)
static void *spare_int;
static uint32_t grbm_cntl;
static uint32_t grbm_idx;
-   bool shadow;
 
scratch_reg0 = adev->rmmio + 
(adev->reg_offset[GC_HWIP][0][mmSCRATCH_REG0_BASE_IDX] + mmSCRATCH_REG0)*4;
scratch_reg1 = adev->rmmio + 
(adev->reg_offset[GC_HWIP][0][mmSCRATCH_REG1_BASE_IDX] + mmSCRATCH_REG1)*4;
@@ -751,10 +750,7 @@ void gfx_v9_0_rlcg_wreg(struct amdgpu_device *adev, u32 
offset, u32 v)
return;
}
 
-   if (offset == grbm_cntl || offset == grbm_idx)
-   shadow = true;
-
-   if (shadow) {
+   if (offset == grbm_cntl || offset == grbm_idx) {
if (offset  == grbm_cntl)
writel(v, scratch_reg2);
else if (offset == grbm_idx)


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

Re: [PATCH -next 023/491] AMD KFD: Use fallthrough;

[Joe Perches]

On Wed, 2020-03-11 at 17:50 -0400, Felix Kuehling wrote:
> On 2020-03-11 12:51 a.m., Joe Perches wrote:
> > Convert the various uses of fallthrough comments to fallthrough;
> > 
> > Done via script
> > Link: 
> > https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/
> 
> The link seems to be broken. This one works: 
> https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git@perches.com/

Thanks.

I neglected to use a backslash on the generating script.
In the script in 0/491,

Link: 
https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git@perches.com/

likely should have been:

Link: 
https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe\@perches.com/


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

[PATCH -next 024/491] AMD DISPLAY CORE: Use fallthrough;

[Joe Perches]

Convert the various uses of fallthrough comments to fallthrough;

Done via script
Link: 
https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/

Signed-off-by: Joe Perches 
---
 drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c | 4 ++--
 drivers/gpu/drm/amd/display/dc/dce/dce_aux.c   | 2 +-
 drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c 
b/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c
index 2f1c958..37fa7b 100644
--- a/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c
+++ b/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c
@@ -267,7 +267,7 @@ static struct atom_display_object_path_v2 *get_bios_object(
&& id.enum_id == obj_id.enum_id)
return 
>object_info_tbl.v1_4->display_path[i];
}
-   /* fall through */
+   fallthrough;
case OBJECT_TYPE_CONNECTOR:
case OBJECT_TYPE_GENERIC:
/* Both Generic and Connector Object ID
@@ -280,7 +280,7 @@ static struct atom_display_object_path_v2 *get_bios_object(
&& id.enum_id == obj_id.enum_id)
return 
>object_info_tbl.v1_4->display_path[i];
}
-   /* fall through */
+   fallthrough;
default:
return NULL;
}
diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c 
b/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
index 68c4049..743042 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_aux.c
@@ -645,7 +645,7 @@ bool dce_aux_transfer_with_retries(struct ddc_service *ddc,
case AUX_TRANSACTION_REPLY_AUX_DEFER:
case AUX_TRANSACTION_REPLY_I2C_OVER_AUX_DEFER:
retry_on_defer = true;
-   /* fall through */
+   fallthrough;
case AUX_TRANSACTION_REPLY_I2C_OVER_AUX_NACK:
if (++aux_defer_retries >= 
AUX_MAX_DEFER_RETRIES) {
goto fail;
diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c 
b/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c
index 8aa937f..51481e 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_mem_input.c
@@ -479,7 +479,7 @@ static void program_grph_pixel_format(
case SURFACE_PIXEL_FORMAT_GRPH_ABGR16161616F:
sign = 1;
floating = 1;
-   /* fall through */
+   fallthrough;
case SURFACE_PIXEL_FORMAT_GRPH_ARGB16161616F: /* shouldn't this get 
float too? */
case SURFACE_PIXEL_FORMAT_GRPH_ARGB16161616:
grph_depth = 3;
-- 
2.24.0

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

[PATCH -next 023/491] AMD KFD: Use fallthrough;

[Joe Perches]

Convert the various uses of fallthrough comments to fallthrough;

Done via script
Link: 
https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/

Signed-off-by: Joe Perches 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
index d6549e..6529ca 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
@@ -79,7 +79,7 @@ static uint32_t get_sdma_rlc_reg_offset(struct amdgpu_device 
*adev,
dev_warn(adev->dev,
 "Invalid sdma engine id (%d), using engine id 0\n",
 engine_id);
-   /* fall through */
+   fallthrough;
case 0:
sdma_engine_reg_base = SOC15_REG_OFFSET(SDMA0, 0,
mmSDMA0_RLC0_RB_CNTL) - mmSDMA0_RLC0_RB_CNTL;
-- 
2.24.0

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

[PATCH -next 025/491] AMD POWERPLAY: Use fallthrough;

[Joe Perches]

Convert the various uses of fallthrough comments to fallthrough;

Done via script
Link: 
https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/

Signed-off-by: Joe Perches 
---
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
index bf04cf..fc5236c 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
@@ -1250,7 +1250,7 @@ static void smu7_set_dpm_event_sources(struct pp_hwmgr 
*hwmgr, uint32_t sources)
switch (sources) {
default:
pr_err("Unknown throttling event sources.");
-   /* fall through */
+   fallthrough;
case 0:
protection = false;
/* src is unused */
@@ -3698,12 +3698,12 @@ static int 
smu7_request_link_speed_change_before_state_change(
data->force_pcie_gen = PP_PCIEGen2;
if (current_link_speed == PP_PCIEGen2)
break;
-   /* fall through */
+   fallthrough;
case PP_PCIEGen2:
if (0 == 
amdgpu_acpi_pcie_performance_request(hwmgr->adev, PCIE_PERF_REQ_GEN2, false))
break;
 #endif
-   /* fall through */
+   fallthrough;
default:
data->force_pcie_gen = 
smu7_get_current_pcie_speed(hwmgr);
break;
-- 
2.24.0

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

[PATCH -next 000/491] treewide: use fallthrough;

[Joe Perches]

There is a new fallthrough pseudo-keyword macro that can be used
to replace the various /* fallthrough */ style comments that are
used to indicate a case label code block is intended to fallthrough
to the next case label block.

See commit 294f69e662d1 ("compiler_attributes.h: Add 'fallthrough'
pseudo keyword for switch/case use")

These patches are intended to allow clang to detect missing
switch/case fallthrough uses.

Do a depth-first pass on the MAINTAINERS file and find the various
F: pattern files and convert the fallthrough comments to fallthrough;
for all files matched by all  F: patterns in in each section.

Done via the perl script below and the previously posted
cvt_fallthrough.pl script.

Link: 
https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/

These patches are based on next-20200310 and are available in

git://repo.or.cz/linux-2.6/trivial-mods.git in branch 20200310_fallthrough_2

$ cat commit_fallthrough.pl
#!/usr/bin/env perl

use sort 'stable';

#
# Reorder a sorted array so file entries are before directory entries
# depends on a trailing / for directories
# so:
#   foo/
#   foo/bar.c
# becomes
#   foo/bar.c
#   foo/
#
sub file_before_directory {
my ($array_ref) = (@_);

my $count = scalar(@$array_ref);

for (my $i = 1; $i < $count; $i++) {
if (substr(@$array_ref[$i - 1], -1) eq '/' &&
substr(@$array_ref[$i], 0, length(@$array_ref[$i - 1])) eq 
@$array_ref[$i - 1]) {
my $string = @$array_ref[$i - 1];
@$array_ref[$i - 1] = @$array_ref[$i];
@$array_ref[$i] = $string;
}
}
}

sub uniq {
my (@parms) = @_;

my %saw;
@parms = grep(!$saw{$_}++, @parms);

return @parms;
}

# Get all the F: file patterns in MAINTAINERS that could be a .[ch] file
my $maintainer_patterns = `grep -P '^F:\\s+' MAINTAINERS`;
my @patterns = split('\n', $maintainer_patterns);
s/^F:\s*// for @patterns;
@patterns = grep(!/^(?:Documentation|tools|scripts)\//, @patterns);
@patterns = grep(!/\.(?:dtsi?|rst|config)$/, @patterns);
@patterns = sort @patterns;
@patterns = sort { $b =~ tr/\//\// cmp $a =~ tr/\//\// } @patterns;
file_before_directory(\@patterns);

my %sections_done;

foreach my $pattern (@patterns) {

# Find the files the pattern matches
my $pattern_files = `git ls-files -- $pattern`;
my @new_patterns = split('\n', $pattern_files);
$pattern_files = join(' ', @new_patterns);
next if ($pattern_files =~ /^\s*$/);

# Find the section the first file matches
my $pattern_file = @new_patterns[0];
my $section_output = `./scripts/get_maintainer.pl --nogit --nogit-fallback 
--sections --pattern-depth=1 $pattern_file`;
my @section = split('\n', $section_output);
my $section_header = @section[0];

print("Section: <$section_header>\n");

# Skip the section if it's already done
print("Already done '$section_header'\n") if 
($sections_done{$section_header});
next if ($sections_done{$section_header}++);

# Find all the .[ch] files in all F: lines in that section
my @new_section;
foreach my $line (@section) {
last if ($line =~ /^\s*$/);
push(@new_section, $line);
}
@section = grep(/^F:/, @new_section);
s/^F:\s*// for @section;

@section = grep(!/^(?:Documentation|tools|scripts)\//, @section);
@section = grep(!/\.(?:dtsi?|rst|config)$/, @section);
@section = sort @section;
@section = uniq(@section);

my $section_files = join(' ', @section);

print("section_files: <$section_files>\n");

next if ($section_files =~ /^\s*$/);

my $cvt_files = `git ls-files -- $section_files`;
my @files = split('\n', $cvt_files);

@files = grep(!/^(?:Documentation|tools|scripts)\//, @files);
@files = grep(!/\.(?:dtsi?|rst|config)$/, @files);
@files = grep(/\.[ch]$/, @files);
@files = sort @files;
@files = uniq(@files);

$cvt_files = join(' ', @files);
print("files: <$cvt_files>\n");

next if (scalar(@files) < 1);

# Convert fallthroughs for all [.ch] files in the section
print("doing cvt_fallthrough.pl -- $cvt_files\n");

`cvt_fallthrough.pl -- $cvt_files`;

# If nothing changed, nothing to commit
`git diff-index --quiet HEAD --`;
next if (!$?);

# Commit the changes
my $fh;

open($fh, "+>", "cvt_fallthrough.commit_msg") or die "$0: can't create 
temporary file: $!\n";
print $fh <https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git@perches.com/
EOF
;
close $fh;

`git commit -s -a -F cvt_fallthrough.commit_msg`;
}

Joe Perches (491):
  MELLANOX ETHERNET INNOVA DRIVERS: Use fallthrough;
  MARVELL OCTEONTX2 RVU ADMIN FUNCTION DRIVER: Use fallthrough;
  MELLANOX MLX5 core VPI driver: Use fallthrough;
  PERFORMANCE EVENTS SUBSYSTEM: Use fallthrough;
  ARM/UNI

Re: [PATCH] drm/amdkfd: Make process queues logs less verbose

[Joe Perches]

On Sun, 2020-02-02 at 00:11 +0100, Julian Sax wrote:
> During normal usage, especially if jobs are started and stopped in rapid
> succession, the kernel log is filled with messages like this:
> 
> [38732.522910] Restoring PASID 0x8003 queues
> [38732.666767] Evicting PASID 0x8003 queues
> [38732.714074] Restoring PASID 0x8003 queues
> [38732.815633] Evicting PASID 0x8003 queues
> [38732.834961] Restoring PASID 0x8003 queues
> [38732.840536] Evicting PASID 0x8003 queues
> [38732.869846] Restoring PASID 0x8003 queues
> [38732.893655] Evicting PASID 0x8003 queues
> [38732.927975] Restoring PASID 0x8003 queues
> 
> According to [1], these messages are expected, but they carry little
> value for the end user, so turn them into debug messages.
> 
> [1] https://github.com/RadeonOpenCompute/ROCm/issues/343

trivia:

> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
[]
> @@ -604,7 +604,7 @@ static int evict_process_queues_nocpsch(struct 
> device_queue_manager *dqm,
>   goto out;
> 
>   pdd = qpd_to_pdd(qpd);
> - pr_info_ratelimited("Evicting PASID 0x%x queues\n",
> + pr_debug_ratelimited("Evicting PASID 0x%x queues\n",
>   pdd->process->pasid);

It would be nicer to realign all the subsequent lines in a
single statement to the now moved open parenthesis.

> 
>   /* Mark all queues as evicted. Deactivate all active queues on
> @@ -650,7 +650,7 @@ static int evict_process_queues_cpsch(struct 
> device_queue_manager *dqm,
>   goto out;
> 
>   pdd = qpd_to_pdd(qpd);
> - pr_info_ratelimited("Evicting PASID 0x%x queues\n",
> + pr_debug_ratelimited("Evicting PASID 0x%x queues\n",
>   pdd->process->pasid);

etc...


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

Re: [PATCH 1/7] drm/amdgpu: remove set but not used variable 'mc_shared_chmap' from 'gfx_v6_0.c' and 'gfx_v7_0.c'

[Joe Perches]

On Wed, 2019-11-13 at 20:44 +0800, yu kuai wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
> 
> drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c: In function
> ‘gfx_v6_0_constants_init’:
> drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c:1579:6: warning: variable
> ‘mc_shared_chmap’ set but not used [-Wunused-but-set-variable]
[]
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
[]
> @@ -1678,7 +1678,6 @@ static void gfx_v6_0_constants_init(struct 
> amdgpu_device *adev)
>  
>   WREG32(mmBIF_FB_EN, BIF_FB_EN__FB_READ_EN_MASK | 
> BIF_FB_EN__FB_WRITE_EN_MASK);
>  
> - mc_shared_chmap = RREG32(mmMC_SHARED_CHMAP);

I do not know the hardware but frequently hardware like
this has read ordering requirements and various registers
can not be read in a random order.

Does removing this read have no effect on the hardware?

>   adev->gfx.config.mc_arb_ramcfg = RREG32(mmMC_ARB_RAMCFG);
>   mc_arb_ramcfg = adev->gfx.config.mc_arb_ramcfg;
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
[]
> @@ -4336,7 +4336,6 @@ static void gfx_v7_0_gpu_early_init(struct 
> amdgpu_device *adev)
>   break;
>   }
>  
> - mc_shared_chmap = RREG32(mmMC_SHARED_CHMAP);
>   adev->gfx.config.mc_arb_ramcfg = RREG32(mmMC_ARB_RAMCFG);
>   mc_arb_ramcfg = adev->gfx.config.mc_arb_ramcfg;

Same question.

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

Re: [PATCH -next] drm/amd/display: Fix old-style declaration

[Joe Perches]

On Mon, 2019-11-11 at 20:28 +0800, YueHaibing wrote:
> Fix a build warning:
> 
> drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:75:1:
>  warning: 'static' is not at beginning of declaration 
> [-Wold-style-declaration]
[]
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
> b/drivers/gpu/drm/amd/display/dc/core/dc.c
[]
> @@ -69,7 +69,7 @@
>  #define DC_LOGGER \
>   dc->ctx->logger
>  
> -const static char DC_BUILD_ID[] = "production-build";
> +static const char DC_BUILD_ID[] = "production-build";

DC_BUILD_ID is used exactly once.
Maybe just use it directly and remove DC_BUILD_ID instead?

---
 drivers/gpu/drm/amd/display/dc/core/dc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 1fdba13..803dc14 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -69,8 +69,6 @@
 #define DC_LOGGER \
dc->ctx->logger
 
-const static char DC_BUILD_ID[] = "production-build";
-
 /**
  * DOC: Overview
  *
@@ -815,7 +813,7 @@ struct dc *dc_create(const struct dc_init_data *init_params)
if (dc->res_pool->dmcu != NULL)
dc->versions.dmcu_version = dc->res_pool->dmcu->dmcu_version;
 
-   dc->build_id = DC_BUILD_ID;
+   dc->build_id = "production-build";
 
DC_LOG_DC("Display Core initialized\n");
 


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

Re: [PATCH] drm/amdgpu/display: make various arrays static, makes object smaller

[Joe Perches]

On Tue, 2019-10-08 at 13:56 +, Harry Wentland wrote:
[]
> > diff --git a/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c 
> > b/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c
> []
> > @@ -2745,7 +2745,7 @@ static enum bp_result bios_get_board_layout_info(
> > struct bios_parser *bp;
> > enum bp_result record_result;
> >  
> > -   const unsigned int slot_index_to_vbios_id[MAX_BOARD_SLOTS] = {
> > +   static const unsigned int slot_index_to_vbios_id[MAX_BOARD_SLOTS] = {
> 
> Won't this break the multi-GPU case where you'll have multiple driver
> instances with different layout?

As the array is read-only, how could that happen?

Re: [PATCH AUTOSEL 4.19 044/167] drm/amdgpu: validate user pitch alignment

[Joe Perches]

On Wed, 2019-09-04 at 08:08 -0400, Sasha Levin wrote:
> it's better to get
> it right rather than to be done quickly :)

That also applies to the initial selection of
patches for the stable trees.

Re: [PATCH][drm-next] drm/amd/powerplay: fix a few spelling mistakes

[Joe Perches]

On Thu, 2019-08-01 at 15:02 -0400, Alex Deucher wrote:
> Applied.  thanks!
> 
> Alex
> 
> On Thu, Aug 1, 2019 at 4:39 AM Colin King  wrote:
> > From: Colin Ian King 
> > 
> > There are a few spelling mistakes "unknow" -> "unknown" and
> > "enabeld" -> "enabled". Fix these.
[]
> > diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c 
> > b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
[]
> > @@ -39,7 +39,7 @@ static const char* __smu_message_names[] = {
> >  const char *smu_get_message_name(struct smu_context *smu, enum 
> > smu_message_type type)
> >  {
> > if (type < 0 || type > SMU_MSG_MAX_COUNT)

This looks like an off-by-one test against
SMU_MSG_MAX_COUNT where type
should be >=

> > -   return "unknow smu message";
> > +   return "unknown smu message";
> > return __smu_message_names[type];
[]
> > @@ -52,7 +52,7 @@ static const char* __smu_feature_names[] = {
> >  const char *smu_get_feature_name(struct smu_context *smu, enum 
> > smu_feature_mask feature)
> >  {
> > if (feature < 0 || feature > SMU_FEATURE_COUNT)

here too

> > -   return "unknow smu feature";
> > +   return "unknown smu feature";
> > return __smu_feature_names[feature];

Perhaps instead it should be against ARRAY_SIZE(__smu_)

Also, the  __SMU_DUMMY_MAP macro is unnecessarily complex.

It might be better to have some direct
index and name struct like

struct enum_name {
int val;
const char *name;
};

And walk that.

Perhaps add a macro like

#define enum_map(e)
{.val = e, .name = #e}

Re: [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init

[Joe Perches]

On Mon, 2019-06-24 at 11:41 +0800, maowenan wrote:
> 
> On 2019/6/23 2:13, Joe Perches wrote:
> > On Sat, 2019-06-22 at 21:05 +0800, Mao Wenan wrote:
> > > There is one warning:
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’:
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ 
> > > set but not used [-Wunused-but-set-variable]
> > >   int ret = 0;
> > []
> > >  v1->v2: change the subject for this patch; change the indenting when it 
> > > calls init_pmu_by_type; use the value 'ret' in
> > >  amdgpu_pmu_init().
> > []
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c 
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
> > []
> > > @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
> > >   case CHIP_VEGA20:
> > >   /* init df */
> > >   ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
> > > -"DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
> > > -DF_V3_6_MAX_COUNTERS);
> > > +"DF", "amdgpu_df", 
> > > PERF_TYPE_AMDGPU_DF,
> > > +
> > > DF_V3_6_MAX_COUNTERS);
> > 
> > trivia:
> > 
> > The indentation change seems superfluous and
> > appears to make the code harder to read.
> > 
> > You could also cc Jonathan Kim who wrote all of this.
> I think this is just display issue in mail format. It is correct that in 
> vi/vim.
> The arguments are line up with '(' after my change.

Use 8 character tabs and try again please.

> @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)$
>  ^Icase CHIP_VEGA20:$
>  ^I^I/* init df */$
>  ^I^Iret = init_pmu_by_type(adev, df_v3_6_attr_groups,$
> -^I^I^I^I   "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,$
> -^I^I^I^I   DF_V3_6_MAX_COUNTERS);$
> +^I^I^I^I^I^I^I   "DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,$
> +^I^I^I^I^I^I^I   DF_V3_6_MAX_COUNTERS);$

Re: [PATCH -next v2] drm/amdgpu: return 'ret' in amdgpu_pmu_init

[Joe Perches]

On Sat, 2019-06-22 at 21:05 +0800, Mao Wenan wrote:
> There is one warning:
> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c: In function ‘amdgpu_pmu_init’:
> drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c:249:6: warning: variable ‘ret’ set 
> but not used [-Wunused-but-set-variable]
>   int ret = 0;
[]
>  v1->v2: change the subject for this patch; change the indenting when it 
> calls init_pmu_by_type; use the value 'ret' in
>  amdgpu_pmu_init().
[]
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c
[]
> @@ -252,8 +252,8 @@ int amdgpu_pmu_init(struct amdgpu_device *adev)
>   case CHIP_VEGA20:
>   /* init df */
>   ret = init_pmu_by_type(adev, df_v3_6_attr_groups,
> -"DF", "amdgpu_df", PERF_TYPE_AMDGPU_DF,
> -DF_V3_6_MAX_COUNTERS);
> +"DF", "amdgpu_df", 
> PERF_TYPE_AMDGPU_DF,
> +
> DF_V3_6_MAX_COUNTERS);

trivia:

The indentation change seems superfluous and
appears to make the code harder to read.

You could also cc Jonathan Kim who wrote all of this.

Re: [PATCH] drm/amd/display: Fix boolean expression in get_surf_rq_param

[Joe Perches]

On Thu, 2019-03-21 at 22:10 -0500, Gustavo A. R. Silva wrote:
> Hi Harry,
> 
> I noticed this patch is already in mainline, but the stable tag
> was removed.  What is the reason for that if this bug is present
> in stable?

It's not a bug, it's just a style issue and
the && use in some compilers it may be slower.

bool when set is 1.


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

Re: [PATCH] mm: convert totalram_pages, totalhigh_pages and managed_pages to atomic.

[Joe Perches]

On Mon, 2018-10-22 at 22:53 +0530, Arun KS wrote:
> Remove managed_page_count_lock spinlock and instead use atomic
> variables.

Perhaps better to define and use macros for the accesses
instead of specific uses of atomic_long_

Something like:

#define totalram_pages()(unsigned 
long)atomic_long_read(&_totalram_pages)
#define totalram_pages_inc()(unsigned long)atomic_long_inc(&_totalram_pages)
#define totalram_pages_dec()(unsigned long)atomic_long_dec(&_totalram_pages)

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

Re: [PATCH] gpu: Consistently use octal not symbolic permissions

[Joe Perches]

On Fri, 2018-05-25 at 09:41 +0300, Jani Nikula wrote:
> On Thu, 24 May 2018, Joe Perches <j...@perches.com> wrote:
> > There is currently a mixture of octal and symbolic permissions uses
> > in files in drivers/gpu/drm and one file in drivers/gpu.
> > 
> > There are ~270 existing octal uses and ~115 S_ uses.
> > 
> > Convert all the S_ symbolic permissions to their octal equivalents
> > as using octal and not symbolic permissions is preferred by many as more
> > readable.
> > 
> > see: https://lkml.org/lkml/2016/8/2/1945
> > 
> > Done with automated conversion via:
> > $ ./scripts/checkpatch.pl -f --types=SYMBOLIC_PERMS --fix-inplace 
> > 
> > Miscellanea:
> > 
> > o Wrapped modified multi-line calls to a single line where appropriate
> > o Realign modified multi-line calls to open parenthesis
> > o drivers/gpu/drm/msm/adreno/a5xx_debugfs.c has a world-writeable
> >   debug permission for "reset" - perhaps that should be modified
> > Signed-off-by: Joe Perches <j...@perches.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c|  2 +-
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 98 
> > +++---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c   |  3 +-
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c|  9 +-
> >  drivers/gpu/drm/armada/armada_debugfs.c|  4 +-
> >  drivers/gpu/drm/drm_debugfs.c  |  6 +-
> >  drivers/gpu/drm/drm_debugfs_crc.c  |  4 +-
> >  drivers/gpu/drm/drm_sysfs.c|  2 +-
> >  drivers/gpu/drm/i915/gvt/firmware.c|  2 +-
> >  drivers/gpu/drm/i915/i915_debugfs.c|  8 +-
> >  drivers/gpu/drm/i915/i915_perf.c   |  2 +-
> >  drivers/gpu/drm/i915/i915_sysfs.c  | 22 ++---
> >  drivers/gpu/drm/i915/intel_pipe_crc.c  |  2 +-
> 
> Please send at least i915 changes separately. There's zero reason to
> make our lives harder for this change.

The idea is to avoid unnecessary multiple patches for
individual trees.

But you could do that via something like:

$ git am --include='drivers/gpu/drm/i915/*' 

cheers, Joe

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

[PATCH] gpu: Consistently use octal not symbolic permissions

[Joe Perches]

There is currently a mixture of octal and symbolic permissions uses
in files in drivers/gpu/drm and one file in drivers/gpu.

There are ~270 existing octal uses and ~115 S_ uses.

Convert all the S_ symbolic permissions to their octal equivalents
as using octal and not symbolic permissions is preferred by many as more
readable.

see: https://lkml.org/lkml/2016/8/2/1945

Done with automated conversion via:
$ ./scripts/checkpatch.pl -f --types=SYMBOLIC_PERMS --fix-inplace 

Miscellanea:

o Wrapped modified multi-line calls to a single line where appropriate
o Realign modified multi-line calls to open parenthesis
o drivers/gpu/drm/msm/adreno/a5xx_debugfs.c has a world-writeable
  debug permission for "reset" - perhaps that should be modified

Signed-off-by: Joe Perches <j...@perches.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c|  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 98 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c   |  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c|  9 +-
 drivers/gpu/drm/armada/armada_debugfs.c|  4 +-
 drivers/gpu/drm/drm_debugfs.c  |  6 +-
 drivers/gpu/drm/drm_debugfs_crc.c  |  4 +-
 drivers/gpu/drm/drm_sysfs.c|  2 +-
 drivers/gpu/drm/i915/gvt/firmware.c|  2 +-
 drivers/gpu/drm/i915/i915_debugfs.c|  8 +-
 drivers/gpu/drm/i915/i915_perf.c   |  2 +-
 drivers/gpu/drm/i915/i915_sysfs.c  | 22 ++---
 drivers/gpu/drm/i915/intel_pipe_crc.c  |  2 +-
 drivers/gpu/drm/msm/adreno/a5xx_debugfs.c  |  5 +-
 drivers/gpu/drm/msm/msm_perf.c |  4 +-
 drivers/gpu/drm/msm/msm_rd.c   |  4 +-
 drivers/gpu/drm/nouveau/nouveau_debugfs.c  |  2 +-
 drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c| 11 ++-
 .../drm/omapdrm/displays/panel-sony-acx565akm.c|  6 +-
 .../drm/omapdrm/displays/panel-tpo-td043mtea1.c| 10 +--
 drivers/gpu/drm/radeon/radeon_pm.c | 26 +++---
 drivers/gpu/drm/radeon/radeon_ttm.c|  4 +-
 drivers/gpu/drm/sti/sti_drv.c  |  2 +-
 drivers/gpu/drm/tinydrm/mipi-dbi.c |  4 +-
 drivers/gpu/drm/ttm/ttm_bo.c   |  2 +-
 drivers/gpu/drm/ttm/ttm_memory.c   | 12 +--
 drivers/gpu/drm/ttm/ttm_page_alloc.c   |  6 +-
 drivers/gpu/drm/ttm/ttm_page_alloc_dma.c   |  6 +-
 drivers/gpu/drm/udl/udl_fb.c   |  4 +-
 drivers/gpu/host1x/debug.c | 12 +--
 30 files changed, 138 insertions(+), 146 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index f5fb93795a69..7b29febff511 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -830,7 +830,7 @@ int amdgpu_debugfs_regs_init(struct amdgpu_device *adev)
 
for (i = 0; i < ARRAY_SIZE(debugfs_regs); i++) {
ent = debugfs_create_file(debugfs_regs_names[i],
- S_IFREG | S_IRUGO, root,
+ S_IFREG | 0444, root,
  adev, debugfs_regs[i]);
if (IS_ERR(ent)) {
for (j = 0; j < i; j++) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index b455da487782..fa55d7e9e784 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -905,39 +905,39 @@ static ssize_t amdgpu_set_pp_power_profile_mode(struct 
device *dev,
return -EINVAL;
 }
 
-static DEVICE_ATTR(power_dpm_state, S_IRUGO | S_IWUSR, amdgpu_get_dpm_state, 
amdgpu_set_dpm_state);
-static DEVICE_ATTR(power_dpm_force_performance_level, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(power_dpm_state, 0644, amdgpu_get_dpm_state, 
amdgpu_set_dpm_state);
+static DEVICE_ATTR(power_dpm_force_performance_level, 0644,
   amdgpu_get_dpm_forced_performance_level,
   amdgpu_set_dpm_forced_performance_level);
-static DEVICE_ATTR(pp_num_states, S_IRUGO, amdgpu_get_pp_num_states, NULL);
-static DEVICE_ATTR(pp_cur_state, S_IRUGO, amdgpu_get_pp_cur_state, NULL);
-static DEVICE_ATTR(pp_force_state, S_IRUGO | S_IWUSR,
-   amdgpu_get_pp_force_state,
-   amdgpu_set_pp_force_state);
-static DEVICE_ATTR(pp_table, S_IRUGO | S_IWUSR,
-   amdgpu_get_pp_table,
-   amdgpu_set_pp_table);
-static DEVICE_ATTR(pp_dpm_sclk, S_IRUGO | S_IWUSR,
-   amdgpu_get_pp_dpm_sclk,
-   amdgpu_set_pp_dpm_sclk);
-static DEVICE_ATTR(pp_dpm_mclk, S_IRUGO | S_IWUSR,
-   amdgpu_get_pp_dpm_mclk,
-   amdgpu_set_pp_dpm_mclk);
-static DEVICE_ATTR(pp_dpm_pcie, S_IRUGO | S_IWUSR,
-   

[trivial PATCH V2] treewide: Align function definition open/close braces

[Joe Perches]

Some functions definitions have either the initial open brace and/or
the closing brace outside of column 1.

Move those braces to column 1.

This allows various function analyzers like gnu complexity to work
properly for these modified functions.

Signed-off-by: Joe Perches <j...@perches.com>
Acked-by: Andy Shevchenko <andy.shevche...@gmail.com>
Acked-by: Paul Moore <p...@paul-moore.com>
Acked-by: Alex Deucher <alexander.deuc...@amd.com>
Acked-by: Dave Chinner <dchin...@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.w...@oracle.com>
Acked-by: Alexandre Belloni <alexandre.bell...@free-electrons.com>
Acked-by: Martin K. Petersen <martin.peter...@oracle.com>
Acked-by: Takashi Iwai <ti...@suse.de>
Acked-by: Mauro Carvalho Chehab <mche...@s-opensource.com>
---

git diff -w still shows no difference.

This patch was sent but December and not applied.

As the trivial maintainer seems not active, it'd be nice if
Andrew Morton picks this up.

V2: Remove fs/xfs/libxfs/xfs_alloc.c as it's updated and remerge the rest

 arch/x86/include/asm/atomic64_32.h   |  2 +-
 drivers/acpi/custom_method.c |  2 +-
 drivers/acpi/fan.c   |  2 +-
 drivers/gpu/drm/amd/display/dc/core/dc.c |  2 +-
 drivers/media/i2c/msp3400-kthreads.c |  2 +-
 drivers/message/fusion/mptsas.c  |  2 +-
 drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c |  2 +-
 drivers/net/wireless/ath/ath9k/xmit.c|  2 +-
 drivers/platform/x86/eeepc-laptop.c  |  2 +-
 drivers/rtc/rtc-ab-b5ze-s3.c |  2 +-
 drivers/scsi/dpt_i2o.c   |  2 +-
 drivers/scsi/sym53c8xx_2/sym_glue.c  |  2 +-
 fs/locks.c   |  2 +-
 fs/ocfs2/stack_user.c|  2 +-
 fs/xfs/xfs_export.c  |  2 +-
 kernel/audit.c   |  6 +++---
 kernel/trace/trace_printk.c  |  4 ++--
 lib/raid6/sse2.c | 14 +++---
 sound/soc/fsl/fsl_dma.c  |  2 +-
 19 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/arch/x86/include/asm/atomic64_32.h 
b/arch/x86/include/asm/atomic64_32.h
index 46e1ef17d92d..92212bf0484f 100644
--- a/arch/x86/include/asm/atomic64_32.h
+++ b/arch/x86/include/asm/atomic64_32.h
@@ -123,7 +123,7 @@ static inline long long arch_atomic64_read(const atomic64_t 
*v)
long long r;
alternative_atomic64(read, "=" (r), "c" (v) : "memory");
return r;
- }
+}
 
 /**
  * arch_atomic64_add_return - add and return
diff --git a/drivers/acpi/custom_method.c b/drivers/acpi/custom_method.c
index b33fba70ec51..a07fbe999eb6 100644
--- a/drivers/acpi/custom_method.c
+++ b/drivers/acpi/custom_method.c
@@ -97,7 +97,7 @@ static void __exit acpi_custom_method_exit(void)
 {
if (cm_dentry)
debugfs_remove(cm_dentry);
- }
+}
 
 module_init(acpi_custom_method_init);
 module_exit(acpi_custom_method_exit);
diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c
index 6cf4988206f2..3563103590c6 100644
--- a/drivers/acpi/fan.c
+++ b/drivers/acpi/fan.c
@@ -219,7 +219,7 @@ fan_set_cur_state(struct thermal_cooling_device *cdev, 
unsigned long state)
return fan_set_state_acpi4(device, state);
else
return fan_set_state(device, state);
- }
+}
 
 static const struct thermal_cooling_device_ops fan_cooling_ops = {
.get_max_state = fan_get_max_state,
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 8394d69b963f..e934326a95d3 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -588,7 +588,7 @@ static void disable_dangling_plane(struct dc *dc, struct 
dc_state *context)
  
**/
 
 struct dc *dc_create(const struct dc_init_data *init_params)
- {
+{
struct dc *dc = kzalloc(sizeof(*dc), GFP_KERNEL);
unsigned int full_pipe_count;
 
diff --git a/drivers/media/i2c/msp3400-kthreads.c 
b/drivers/media/i2c/msp3400-kthreads.c
index 4dd01e9f553b..dc6cb8d475b3 100644
--- a/drivers/media/i2c/msp3400-kthreads.c
+++ b/drivers/media/i2c/msp3400-kthreads.c
@@ -885,7 +885,7 @@ static int msp34xxg_modus(struct i2c_client *client)
 }
 
 static void msp34xxg_set_source(struct i2c_client *client, u16 reg, int in)
- {
+{
struct msp_state *state = to_state(i2c_get_clientdata(client));
int source, matrix;
 
diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
index 439ee9c5f535..231f3a1e27bf 100644
--- a/drivers/message/fusion/mptsas.c
+++ b/drivers/message/fusion/mptsas.c
@@ -2967,7 +2967,7 @@ mp

Re: [PATCH] drm/amd/pp: use mlck_table.count for array loop index limit

[Joe Perches]

On Wed, 2018-03-21 at 18:26 +, Colin King wrote:
> From: Colin Ian King 
> 
> The for-loops process data in the mclk_table but use slck_table.count
> as the loop index limit.  I believe these are cut-n-paste errors from
> the previous almost identical loops as indicated by static analysis.
> Fix these.

Nice tool.

> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
[]
> @@ -855,7 +855,7 @@ static int smu7_odn_initial_default_setting(struct 
> pp_hwmgr *hwmgr)
>  
>   odn_table->odn_memory_clock_dpm_levels.num_of_pl =
>   
> data->golden_dpm_table.mclk_table.count;
> - for (i=0; igolden_dpm_table.sclk_table.count; i++) {
> + for (i=0; igolden_dpm_table.mclk_table.count; i++) {
>   odn_table->odn_memory_clock_dpm_levels.entries[i].clock =
>   
> data->golden_dpm_table.mclk_table.dpm_levels[i].value;
>   odn_table->odn_memory_clock_dpm_levels.entries[i].enabled = 
> true;

Probably more sensible to use temporaries too.
Maybe something like the below (also trivially reduces object size)
---
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
index df2a312ca6c9..339b897146af 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
@@ -834,6 +834,7 @@ static int smu7_odn_initial_default_setting(struct pp_hwmgr 
*hwmgr)
 
struct phm_ppt_v1_clock_voltage_dependency_table *dep_sclk_table;
struct phm_ppt_v1_clock_voltage_dependency_table *dep_mclk_table;
+   struct phm_odn_performance_level *entries;
 
if (table_info == NULL)
return -EINVAL;
@@ -843,11 +844,11 @@ static int smu7_odn_initial_default_setting(struct 
pp_hwmgr *hwmgr)
 
odn_table->odn_core_clock_dpm_levels.num_of_pl =

data->golden_dpm_table.sclk_table.count;
+   entries = odn_table->odn_core_clock_dpm_levels.entries;
for (i=0; igolden_dpm_table.sclk_table.count; i++) {
-   odn_table->odn_core_clock_dpm_levels.entries[i].clock =
-   
data->golden_dpm_table.sclk_table.dpm_levels[i].value;
-   odn_table->odn_core_clock_dpm_levels.entries[i].enabled = true;
-   odn_table->odn_core_clock_dpm_levels.entries[i].vddc = 
dep_sclk_table->entries[i].vddc;
+   entries[i].clock = 
data->golden_dpm_table.sclk_table.dpm_levels[i].value;
+   entries[i].enabled = true;
+   entries[i].vddc = dep_sclk_table->entries[i].vddc;
}
 
smu7_get_voltage_dependency_table(dep_sclk_table,
@@ -855,11 +856,11 @@ static int smu7_odn_initial_default_setting(struct 
pp_hwmgr *hwmgr)
 
odn_table->odn_memory_clock_dpm_levels.num_of_pl =

data->golden_dpm_table.mclk_table.count;
-   for (i=0; igolden_dpm_table.sclk_table.count; i++) {
-   odn_table->odn_memory_clock_dpm_levels.entries[i].clock =
-   
data->golden_dpm_table.mclk_table.dpm_levels[i].value;
-   odn_table->odn_memory_clock_dpm_levels.entries[i].enabled = 
true;
-   odn_table->odn_memory_clock_dpm_levels.entries[i].vddc = 
dep_mclk_table->entries[i].vddc;
+   entries = odn_table->odn_memory_clock_dpm_levels.entries;
+   for (i=0; igolden_dpm_table.mclk_table.count; i++) {
+   entries[i].clock = 
data->golden_dpm_table.mclk_table.dpm_levels[i].value;
+   entries[i].enabled = true;
+   entries[i].vddc = dep_mclk_table->entries[i].vddc;
}
 
smu7_get_voltage_dependency_table(dep_mclk_table,
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] fix double ;;s in code

[Joe Perches]

On Tue, 2018-02-20 at 17:19 +1100, Michael Ellerman wrote:
> Daniel Vetter  writes:
> > On Sun, Feb 18, 2018 at 11:00:56AM +0100, Christophe LEROY wrote:
> > > Le 17/02/2018 à 22:19, Pavel Machek a écrit :
> > > > 
> > > > Fix double ;;'s in code.
> > > > 
> > > > Signed-off-by: Pavel Machek 
> > > 
> > > A summary of the files modified on top of the patch would help understand
> > > the impact.
> > > 
> > > A maybe there should be one patch by area, eg one for each arch specific
> > > modif and one for drivers/ and one for block/ ?
> > 
> > Yeah, pls split this into one patch per area, with a suitable patch
> > subject prefix. Look at git log of each file to get a feeling for what's
> > the standard in each area.
> 
> This part is actually pretty annoying.
> 
> I hacked up a script (below) which seems to do a reasonable job in most
> cases.

Here was another suggestion from awhile ago
https://lkml.org/lkml/2010/11/16/245

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

[trivial PATCH] treewide: Align function definition open/close braces

[Joe Perches]

Some functions definitions have either the initial open brace and/or
the closing brace outside of column 1.

Move those braces to column 1.

This allows various function analyzers like gnu complexity to work
properly for these modified functions.

Miscellanea:

o Remove extra trailing ; and blank line from xfs_agf_verify

Signed-off-by: Joe Perches <j...@perches.com>
---
git diff -w shows no difference other than the above 'Miscellanea'

(this is against -next, but it applies against Linus' tree
 with a couple offsets)

 arch/x86/include/asm/atomic64_32.h   |  2 +-
 drivers/acpi/custom_method.c |  2 +-
 drivers/acpi/fan.c   |  2 +-
 drivers/gpu/drm/amd/display/dc/core/dc.c |  2 +-
 drivers/media/i2c/msp3400-kthreads.c |  2 +-
 drivers/message/fusion/mptsas.c  |  2 +-
 drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c |  2 +-
 drivers/net/wireless/ath/ath9k/xmit.c|  2 +-
 drivers/platform/x86/eeepc-laptop.c  |  2 +-
 drivers/rtc/rtc-ab-b5ze-s3.c |  2 +-
 drivers/scsi/dpt_i2o.c   |  2 +-
 drivers/scsi/sym53c8xx_2/sym_glue.c  |  2 +-
 fs/locks.c   |  2 +-
 fs/ocfs2/stack_user.c|  2 +-
 fs/xfs/libxfs/xfs_alloc.c|  5 ++---
 fs/xfs/xfs_export.c  |  2 +-
 kernel/audit.c   |  6 +++---
 kernel/trace/trace_printk.c  |  4 ++--
 lib/raid6/sse2.c | 14 +++---
 sound/soc/fsl/fsl_dma.c  |  2 +-
 20 files changed, 30 insertions(+), 31 deletions(-)

diff --git a/arch/x86/include/asm/atomic64_32.h 
b/arch/x86/include/asm/atomic64_32.h
index 97c46b8169b7..d4d4883080fa 100644
--- a/arch/x86/include/asm/atomic64_32.h
+++ b/arch/x86/include/asm/atomic64_32.h
@@ -122,7 +122,7 @@ static inline long long atomic64_read(const atomic64_t *v)
long long r;
alternative_atomic64(read, "=" (r), "c" (v) : "memory");
return r;
- }
+}
 
 /**
  * atomic64_add_return - add and return
diff --git a/drivers/acpi/custom_method.c b/drivers/acpi/custom_method.c
index c68e72414a67..e967c1173ba3 100644
--- a/drivers/acpi/custom_method.c
+++ b/drivers/acpi/custom_method.c
@@ -94,7 +94,7 @@ static void __exit acpi_custom_method_exit(void)
 {
if (cm_dentry)
debugfs_remove(cm_dentry);
- }
+}
 
 module_init(acpi_custom_method_init);
 module_exit(acpi_custom_method_exit);
diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c
index 6cf4988206f2..3563103590c6 100644
--- a/drivers/acpi/fan.c
+++ b/drivers/acpi/fan.c
@@ -219,7 +219,7 @@ fan_set_cur_state(struct thermal_cooling_device *cdev, 
unsigned long state)
return fan_set_state_acpi4(device, state);
else
return fan_set_state(device, state);
- }
+}
 
 static const struct thermal_cooling_device_ops fan_cooling_ops = {
.get_max_state = fan_get_max_state,
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index d1488d5ee028..1e0d1e7c5324 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -461,7 +461,7 @@ static void disable_dangling_plane(struct dc *dc, struct 
dc_state *context)
  
**/
 
 struct dc *dc_create(const struct dc_init_data *init_params)
- {
+{
struct dc *dc = kzalloc(sizeof(*dc), GFP_KERNEL);
unsigned int full_pipe_count;
 
diff --git a/drivers/media/i2c/msp3400-kthreads.c 
b/drivers/media/i2c/msp3400-kthreads.c
index 4dd01e9f553b..dc6cb8d475b3 100644
--- a/drivers/media/i2c/msp3400-kthreads.c
+++ b/drivers/media/i2c/msp3400-kthreads.c
@@ -885,7 +885,7 @@ static int msp34xxg_modus(struct i2c_client *client)
 }
 
 static void msp34xxg_set_source(struct i2c_client *client, u16 reg, int in)
- {
+{
struct msp_state *state = to_state(i2c_get_clientdata(client));
int source, matrix;
 
diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
index 345f6035599e..69a62d23514b 100644
--- a/drivers/message/fusion/mptsas.c
+++ b/drivers/message/fusion/mptsas.c
@@ -2968,7 +2968,7 @@ mptsas_exp_repmanufacture_info(MPT_ADAPTER *ioc,
mutex_unlock(>sas_mgmt.mutex);
 out:
return ret;
- }
+}
 
 static void
 mptsas_parse_device_info(struct sas_identify *identify,
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c 
b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
index 3dd973475125..0ea141ece19e 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
@@ -603,7 +603,7 @@ static s

Re: [PATCH 0/4] treewide: Fix line continuation formats

[Joe Perches]

On Thu, 2017-11-16 at 12:11 -0500, Mimi Zohar wrote:
> On Thu, 2017-11-16 at 07:27 -0800, Joe Perches wrote:
> > Avoid using line continations in formats as that causes unexpected
> > output.
> 
> Is having lines greater than 80 characters the preferred method?

yes.

>  Could you add quotes before the backlash and before the first word on
> the next line instead?

coalesced formats are preferred.

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

[PATCH 2/4] drm: amd: Fix line continuation formats

[Joe Perches]

Line continuations with excess spacing causes unexpected output.

Miscellanea:

o Added missing '\n' to a few of the coalesced pr_ formats

Signed-off-by: Joe Perches <j...@perches.com>
---
 drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c   | 11 -
 .../amd/powerplay/hwmgr/process_pptables_v1_0.c|  6 ++---
 drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 27 --
 drivers/gpu/drm/amd/powerplay/smumgr/ci_smumgr.c   |  6 ++---
 .../gpu/drm/amd/powerplay/smumgr/iceland_smumgr.c  |  9 +++-
 .../gpu/drm/amd/powerplay/smumgr/vega10_smumgr.c   |  6 ++---
 6 files changed, 22 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index ced42484dcfc..6743786afcce 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -220,8 +220,7 @@ static void dpcd_set_lt_pattern_and_lane_settings(
size_in_bytes);
 
dm_logger_write(link->ctx->logger, LOG_HW_LINK_TRAINING,
-   "%s:\n %x VS set = %x  PE set = %x \
-   max VS Reached = %x  max PE Reached = %x\n",
+   "%s:\n %x VS set = %x  PE set = %x max VS Reached = %x  max PE 
Reached = %x\n",
__func__,
DP_TRAINING_LANE0_SET,
dpcd_lane[0].bits.VOLTAGE_SWING_SET,
@@ -558,8 +557,7 @@ static void dpcd_set_lane_settings(
*/
 
dm_logger_write(link->ctx->logger, LOG_HW_LINK_TRAINING,
-   "%s\n %x VS set = %x  PE set = %x \
-   max VS Reached = %x  max PE Reached = %x\n",
+   "%s\n %x VS set = %x  PE set = %x max VS Reached = %x  max PE 
Reached = %x\n",
__func__,
DP_TRAINING_LANE0_SET,
dpcd_lane[0].bits.VOLTAGE_SWING_SET,
@@ -872,9 +870,8 @@ static bool perform_clock_recovery_sequence(
if (retry_count >= LINK_TRAINING_MAX_CR_RETRY) {
ASSERT(0);
dm_logger_write(link->ctx->logger, LOG_ERROR,
-   "%s: Link Training Error, could not \
-get CR after %d tries. \
-   Possibly voltage swing issue", __func__,
+   "%s: Link Training Error, could not get CR after %d 
tries. Possibly voltage swing issue",
+   __func__,
LINK_TRAINING_MAX_CR_RETRY);
 
}
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/process_pptables_v1_0.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/process_pptables_v1_0.c
index d1af1483c69b..813f827e4270 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/process_pptables_v1_0.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/process_pptables_v1_0.c
@@ -523,8 +523,7 @@ static int get_pcie_table(
if ((uint32_t)atom_pcie_table->ucNumEntries <= pcie_count)
pcie_count = (uint32_t)atom_pcie_table->ucNumEntries;
else
-   pr_err("Number of Pcie Entries exceed the number of 
SCLK Dpm Levels! \
-   Disregarding the excess entries... \n");
+   pr_err("Number of Pcie Entries exceed the number of 
SCLK Dpm Levels! Disregarding the excess entries...\n");
 
pcie_table->count = pcie_count;
for (i = 0; i < pcie_count; i++) {
@@ -563,8 +562,7 @@ static int get_pcie_table(
if ((uint32_t)atom_pcie_table->ucNumEntries <= pcie_count)
pcie_count = (uint32_t)atom_pcie_table->ucNumEntries;
else
-   pr_err("Number of Pcie Entries exceed the number of 
SCLK Dpm Levels! \
-   Disregarding the excess entries... \n");
+   pr_err("Number of Pcie Entries exceed the number of 
SCLK Dpm Levels! Disregarding the excess entries...\n");
 
pcie_table->count = pcie_count;
 
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
index 4f79c21f27ed..9599fe0ba779 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
@@ -546,8 +546,7 @@ static void vega10_patch_with_vdd_leakage(struct pp_hwmgr 
*hwmgr,
}
 
if (*voltage > ATOM_VIRTUAL_VOLTAGE_ID0)
-   pr_info("Voltage value looks like a Leakage ID \
-   but it's not patched\n");
+   pr_info("Voltage value looks like a Leakage ID but it's not 
patched\n");
 }
 
 /**
@@ -701,18 +700,14 @@ static int 
vega10_set_private_data_based_on_pptable(struct pp_hwmgr *hwmgr)
table_info->vdd_dep_on_mclk;
 
PP_ASSERT_WITH_CODE(allowed_sclk_vdd_ta

Re: [PATCH 2/4] drm: amd: Fix line continuation formats

[Joe Perches]

On Thu, 2017-11-16 at 10:38 -0500, Harry Wentland wrote:
> On 2017-11-16 10:27 AM, Joe Perches wrote:
> > Line continuations with excess spacing causes unexpected output.
[]
> > @@ -872,9 +870,8 @@ static bool perform_clock_recovery_sequence(
> > if (retry_count >= LINK_TRAINING_MAX_CR_RETRY) {
> > ASSERT(0);
> > dm_logger_write(link->ctx->logger, LOG_ERROR,
> > -   "%s: Link Training Error, could not \
> > -get CR after %d tries. \
> > -   Possibly voltage swing issue", __func__,
> > +   "%s: Link Training Error, could not get CR after %d 
> > tries. Possibly voltage swing issue",
> 
> Would probably be good to add a '\n' here as well but that's not the main 
> intention of this patch.

About 1/4 of the dm_logger_write calls are missing
newlines and I think it should be a separate patch.

I encourage you to fix them one day.

> Reviewed-by: Harry Wentland <harry.wentl...@amd.com>

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

[PATCH 1/3] gpu: drm: amd/radeon: Convert printk(KERN_ to pr_

[Joe Perches]

Use a more common logging style.

Miscellanea:

o Coalesce formats and realign arguments
o Neaten a few macros now using pr_

Signed-off-by: Joe Perches <j...@perches.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h|  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_afmt.c   |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c   |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c |  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_test.c   |  2 +-
 drivers/gpu/drm/amd/amdgpu/atom.c  | 44 -
 drivers/gpu/drm/amd/amdgpu/ci_dpm.c|  4 +-
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c  |  4 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c  |  4 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c  |  4 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c  |  8 ++--
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c  |  8 ++--
 drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c |  4 +-
 drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c |  4 +-
 drivers/gpu/drm/amd/include/amd_pcie_helpers.h |  4 +-
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   |  2 +-
 drivers/gpu/drm/amd/powerplay/inc/pp_debug.h   |  2 +-
 drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.c|  4 +-
 drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.c | 14 +++---
 .../gpu/drm/amd/powerplay/smumgr/polaris10_smc.c   |  4 +-
 drivers/gpu/drm/amd/powerplay/smumgr/tonga_smc.c   |  4 +-
 drivers/gpu/drm/radeon/atom.c  | 46 --
 drivers/gpu/drm/radeon/cik.c   | 56 --
 drivers/gpu/drm/radeon/evergreen.c |  2 +-
 drivers/gpu/drm/radeon/evergreen_cs.c  |  7 ++-
 drivers/gpu/drm/radeon/ni.c| 22 +++--
 drivers/gpu/drm/radeon/r100.c  | 18 +++
 drivers/gpu/drm/radeon/r200.c  |  3 +-
 drivers/gpu/drm/radeon/r300.c  | 13 ++---
 drivers/gpu/drm/radeon/r420.c  |  9 ++--
 drivers/gpu/drm/radeon/r520.c  |  3 +-
 drivers/gpu/drm/radeon/r600.c  | 21 +++-
 drivers/gpu/drm/radeon/r600_cs.c   |  7 ++-
 drivers/gpu/drm/radeon/radeon.h|  3 +-
 drivers/gpu/drm/radeon/radeon_atpx_handler.c   |  4 +-
 drivers/gpu/drm/radeon/radeon_audio.c  |  4 +-
 drivers/gpu/drm/radeon/radeon_clocks.c |  2 +-
 drivers/gpu/drm/radeon/radeon_device.c |  8 ++--
 drivers/gpu/drm/radeon/radeon_fb.c |  3 +-
 drivers/gpu/drm/radeon/radeon_gem.c|  4 +-
 drivers/gpu/drm/radeon/radeon_test.c   |  6 +--
 drivers/gpu/drm/radeon/rs400.c |  4 +-
 drivers/gpu/drm/radeon/rs690.c |  3 +-
 drivers/gpu/drm/radeon/rv515.c |  9 ++--
 drivers/gpu/drm/radeon/si.c| 45 ++---
 46 files changed, 172 insertions(+), 268 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index c1b913541739..3f636632c289 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1515,7 +1515,8 @@ void amdgpu_mm_wdoorbell(struct amdgpu_device *adev, u32 
index, u32 v);
  */
 #define RREG32(reg) amdgpu_mm_rreg(adev, (reg), false)
 #define RREG32_IDX(reg) amdgpu_mm_rreg(adev, (reg), true)
-#define DREG32(reg) printk(KERN_INFO "REGISTER: " #reg " : 0x%08X\n", 
amdgpu_mm_rreg(adev, (reg), false))
+#define DREG32(reg) pr_info("REGISTER: " #reg " : 0x%08X\n",   \
+   amdgpu_mm_rreg(adev, (reg), false))
 #define WREG32(reg, v) amdgpu_mm_wreg(adev, (reg), (v), false)
 #define WREG32_IDX(reg, v) amdgpu_mm_wreg(adev, (reg), (v), true)
 #define REG_SET(FIELD, v) (((v) << FIELD##_SHIFT) & FIELD##_MASK)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_afmt.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_afmt.c
index 857ba0897159..3889486f71fe 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_afmt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_afmt.c
@@ -74,9 +74,9 @@ static void amdgpu_afmt_calc_cts(uint32_t clock, int *CTS, 
int *N, int freq)
 
/* Check that we are in spec (not always possible) */
if (n < (128*freq/1500))
-   printk(KERN_WARNING "Calculated ACR N value is too small. You 
may experience audio problems.\n");
+   pr_warn("Calculated ACR N value is too small. You may 
experience audio problems.\n");
if (n > (128*freq/300))
-   printk(KERN_WARNING "Calculated ACR N value is too large. You 
may experience audio problems.\n");
+   pr_warn("Calculated ACR N value is too large. You may 
experience audio problems.\n");
 
*N = n;
*

[PATCH 0/3] gpu: drm: Convert printk(KERN_ to pr_

[Joe Perches]

Broken up for Daniel Vetter

Joe Perches (3):
  gpu: drm: amd/radeon: Convert printk(KERN_ to pr_
  gpu: drm: core: Convert printk(KERN_ to pr_
  gpu: drm: drivers: Convert printk(KERN_ to pr_

 drivers/gpu/drm/amd/amdgpu/amdgpu.h|  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_afmt.c   |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c   |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c |  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_test.c   |  2 +-
 drivers/gpu/drm/amd/amdgpu/atom.c  | 44 -
 drivers/gpu/drm/amd/amdgpu/ci_dpm.c|  4 +-
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c  |  4 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c  |  4 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c  |  4 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c  |  8 ++--
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c  |  8 ++--
 drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c |  4 +-
 drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c |  4 +-
 drivers/gpu/drm/amd/include/amd_pcie_helpers.h |  4 +-
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   |  2 +-
 drivers/gpu/drm/amd/powerplay/inc/pp_debug.h   |  2 +-
 drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.c|  4 +-
 drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.c | 14 +++---
 .../gpu/drm/amd/powerplay/smumgr/polaris10_smc.c   |  4 +-
 drivers/gpu/drm/amd/powerplay/smumgr/tonga_smc.c   |  4 +-
 drivers/gpu/drm/drm_cache.c| 12 ++---
 drivers/gpu/drm/drm_edid.c |  4 +-
 drivers/gpu/drm/drm_ioc32.c|  3 +-
 drivers/gpu/drm/gma500/cdv_intel_lvds.c|  9 ++--
 drivers/gpu/drm/gma500/oaktrail_lvds.c | 18 +++
 drivers/gpu/drm/gma500/psb_drv.h   |  5 +-
 drivers/gpu/drm/gma500/psb_intel_lvds.c|  7 ++-
 drivers/gpu/drm/i915/i915_sw_fence.c   |  8 ++--
 drivers/gpu/drm/mgag200/mgag200_mode.c |  2 +-
 drivers/gpu/drm/msm/msm_drv.c  |  2 +-
 drivers/gpu/drm/nouveau/nouveau_acpi.c |  7 +--
 drivers/gpu/drm/nouveau/nouveau_vga.c  |  4 +-
 drivers/gpu/drm/nouveau/nv50_display.c | 22 -
 drivers/gpu/drm/nouveau/nvkm/core/mm.c | 10 ++--
 drivers/gpu/drm/omapdrm/dss/dsi.c  | 17 ---
 drivers/gpu/drm/omapdrm/dss/dss.c  |  3 +-
 drivers/gpu/drm/omapdrm/dss/dss.h  | 15 +++---
 drivers/gpu/drm/omapdrm/omap_gem.c |  5 +-
 drivers/gpu/drm/r128/r128_cce.c|  7 ++-
 drivers/gpu/drm/radeon/atom.c  | 46 --
 drivers/gpu/drm/radeon/cik.c   | 56 --
 drivers/gpu/drm/radeon/evergreen.c |  2 +-
 drivers/gpu/drm/radeon/evergreen_cs.c  |  7 ++-
 drivers/gpu/drm/radeon/ni.c| 22 +++--
 drivers/gpu/drm/radeon/r100.c  | 18 +++
 drivers/gpu/drm/radeon/r200.c  |  3 +-
 drivers/gpu/drm/radeon/r300.c  | 13 ++---
 drivers/gpu/drm/radeon/r420.c  |  9 ++--
 drivers/gpu/drm/radeon/r520.c  |  3 +-
 drivers/gpu/drm/radeon/r600.c  | 21 +++-
 drivers/gpu/drm/radeon/r600_cs.c   |  7 ++-
 drivers/gpu/drm/radeon/radeon.h|  3 +-
 drivers/gpu/drm/radeon/radeon_atpx_handler.c   |  4 +-
 drivers/gpu/drm/radeon/radeon_audio.c  |  4 +-
 drivers/gpu/drm/radeon/radeon_clocks.c |  2 +-
 drivers/gpu/drm/radeon/radeon_device.c |  8 ++--
 drivers/gpu/drm/radeon/radeon_fb.c |  3 +-
 drivers/gpu/drm/radeon/radeon_gem.c|  4 +-
 drivers/gpu/drm/radeon/radeon_test.c   |  6 +--
 drivers/gpu/drm/radeon/rs400.c |  4 +-
 drivers/gpu/drm/radeon/rs690.c |  3 +-
 drivers/gpu/drm/radeon/rv515.c |  9 ++--
 drivers/gpu/drm/radeon/si.c| 45 ++---
 drivers/gpu/drm/ttm/ttm_bo.c   |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_fence.c  |  6 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c  |  3 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_resource.c   |  4 +-
 69 files changed, 253 insertions(+), 362 deletions(-)

-- 
2.10.0.rc2.1.g053435c

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

[PATCH 2/2] gpu: drm: Convert printk(KERN_ to pr_

[Joe Perches]

Use a more common logging style.

Miscellanea:

o Coalesce formats and realign arguments
o Neaten a few macros now using pr_

Signed-off-by: Joe Perches <j...@perches.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h|  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_afmt.c   |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c   |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c |  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_test.c   |  2 +-
 drivers/gpu/drm/amd/amdgpu/atom.c  | 44 -
 drivers/gpu/drm/amd/amdgpu/ci_dpm.c|  4 +-
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c  |  4 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c  |  4 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c  |  4 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c  |  8 ++--
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c  |  8 ++--
 drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c |  4 +-
 drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c |  4 +-
 drivers/gpu/drm/amd/include/amd_pcie_helpers.h |  4 +-
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   |  2 +-
 drivers/gpu/drm/amd/powerplay/inc/pp_debug.h   |  2 +-
 drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.c|  4 +-
 drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.c | 14 +++---
 .../gpu/drm/amd/powerplay/smumgr/polaris10_smc.c   |  4 +-
 drivers/gpu/drm/amd/powerplay/smumgr/tonga_smc.c   |  4 +-
 drivers/gpu/drm/drm_cache.c| 12 ++---
 drivers/gpu/drm/drm_edid.c |  4 +-
 drivers/gpu/drm/drm_ioc32.c|  3 +-
 drivers/gpu/drm/gma500/cdv_intel_lvds.c|  9 ++--
 drivers/gpu/drm/gma500/oaktrail_lvds.c | 18 +++
 drivers/gpu/drm/gma500/psb_drv.h   |  5 +-
 drivers/gpu/drm/gma500/psb_intel_lvds.c|  7 ++-
 drivers/gpu/drm/i915/i915_sw_fence.c   |  8 ++--
 drivers/gpu/drm/mgag200/mgag200_mode.c |  2 +-
 drivers/gpu/drm/msm/msm_drv.c  |  2 +-
 drivers/gpu/drm/nouveau/nouveau_acpi.c |  7 +--
 drivers/gpu/drm/nouveau/nouveau_vga.c  |  4 +-
 drivers/gpu/drm/nouveau/nv50_display.c | 22 -
 drivers/gpu/drm/nouveau/nvkm/core/mm.c | 10 ++--
 drivers/gpu/drm/omapdrm/dss/dsi.c  | 17 ---
 drivers/gpu/drm/omapdrm/dss/dss.c  |  3 +-
 drivers/gpu/drm/omapdrm/dss/dss.h  | 15 +++---
 drivers/gpu/drm/omapdrm/omap_gem.c |  5 +-
 drivers/gpu/drm/r128/r128_cce.c|  7 ++-
 drivers/gpu/drm/radeon/atom.c  | 46 --
 drivers/gpu/drm/radeon/cik.c   | 56 --
 drivers/gpu/drm/radeon/evergreen.c |  2 +-
 drivers/gpu/drm/radeon/evergreen_cs.c  |  7 ++-
 drivers/gpu/drm/radeon/ni.c| 22 +++--
 drivers/gpu/drm/radeon/r100.c  | 18 +++
 drivers/gpu/drm/radeon/r200.c  |  3 +-
 drivers/gpu/drm/radeon/r300.c  | 13 ++---
 drivers/gpu/drm/radeon/r420.c  |  9 ++--
 drivers/gpu/drm/radeon/r520.c  |  3 +-
 drivers/gpu/drm/radeon/r600.c  | 21 +++-
 drivers/gpu/drm/radeon/r600_cs.c   |  7 ++-
 drivers/gpu/drm/radeon/radeon.h|  3 +-
 drivers/gpu/drm/radeon/radeon_atpx_handler.c   |  4 +-
 drivers/gpu/drm/radeon/radeon_audio.c  |  4 +-
 drivers/gpu/drm/radeon/radeon_clocks.c |  2 +-
 drivers/gpu/drm/radeon/radeon_device.c |  8 ++--
 drivers/gpu/drm/radeon/radeon_fb.c |  3 +-
 drivers/gpu/drm/radeon/radeon_gem.c|  4 +-
 drivers/gpu/drm/radeon/radeon_test.c   |  6 +--
 drivers/gpu/drm/radeon/rs400.c |  4 +-
 drivers/gpu/drm/radeon/rs690.c |  3 +-
 drivers/gpu/drm/radeon/rv515.c |  9 ++--
 drivers/gpu/drm/radeon/si.c| 45 ++---
 drivers/gpu/drm/ttm/ttm_bo.c   |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_fence.c  |  6 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c  |  3 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_resource.c   |  4 +-
 69 files changed, 253 insertions(+), 362 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index c1b913541739..3f636632c289 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1515,7 +1515,8 @@ void amdgpu_mm_wdoorbell(struct amdgpu_device *adev, u32 
index, u32 v);
  */
 #define RREG32(reg) amdgpu_mm_rreg(adev, (reg), false)
 #define RREG32_IDX(reg) amdgpu_mm_rreg(adev, (reg), true)
-#define DREG32(reg) printk(KERN_INFO "REGIS

[PATCH 0/2] gpu: drm: Use pr_cont and neaten logging

[Joe Perches]

Joe Perches (2):
  drm: Use pr_cont where appropriate
  gpu: drm: Convert printk(KERN_ to pr_

 drivers/gpu/drm/amd/amdgpu/amdgpu.h|  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_afmt.c   |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c   |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.c| 70 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c |  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_test.c   |  2 +-
 drivers/gpu/drm/amd/amdgpu/atom.c  | 44 ++
 drivers/gpu/drm/amd/amdgpu/ci_dpm.c|  4 +-
 drivers/gpu/drm/amd/amdgpu/cik_sdma.c  |  4 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c  |  4 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c  |  4 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c  |  8 +--
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c  |  8 +--
 drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c |  4 +-
 drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c |  4 +-
 drivers/gpu/drm/amd/include/amd_pcie_helpers.h |  4 +-
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   |  2 +-
 drivers/gpu/drm/amd/powerplay/inc/pp_debug.h   |  2 +-
 drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.c|  4 +-
 drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.c | 14 ++---
 .../gpu/drm/amd/powerplay/smumgr/polaris10_smc.c   |  4 +-
 drivers/gpu/drm/amd/powerplay/smumgr/tonga_smc.c   |  4 +-
 drivers/gpu/drm/drm_cache.c| 12 ++--
 drivers/gpu/drm/drm_edid.c |  4 +-
 drivers/gpu/drm/drm_ioc32.c|  3 +-
 drivers/gpu/drm/gma500/cdv_intel_lvds.c|  9 ++-
 drivers/gpu/drm/gma500/oaktrail_lvds.c | 18 +++---
 drivers/gpu/drm/gma500/psb_drv.h   |  5 +-
 drivers/gpu/drm/gma500/psb_intel_lvds.c|  7 +--
 drivers/gpu/drm/i915/i915_sw_fence.c   |  8 +--
 drivers/gpu/drm/mgag200/mgag200_mode.c |  2 +-
 drivers/gpu/drm/msm/msm_drv.c  |  2 +-
 drivers/gpu/drm/nouveau/nouveau_acpi.c |  7 ++-
 drivers/gpu/drm/nouveau/nouveau_vga.c  |  4 +-
 drivers/gpu/drm/nouveau/nv50_display.c | 22 +++
 drivers/gpu/drm/nouveau/nvkm/core/mm.c | 10 +--
 drivers/gpu/drm/omapdrm/dss/dsi.c  | 17 +++---
 drivers/gpu/drm/omapdrm/dss/dss.c  |  3 +-
 drivers/gpu/drm/omapdrm/dss/dss.h  | 15 ++---
 drivers/gpu/drm/omapdrm/omap_gem.c |  5 +-
 drivers/gpu/drm/r128/r128_cce.c|  7 +--
 drivers/gpu/drm/radeon/atom.c  | 46 ++
 drivers/gpu/drm/radeon/cik.c   | 56 ++---
 drivers/gpu/drm/radeon/evergreen.c |  2 +-
 drivers/gpu/drm/radeon/evergreen_cs.c  |  7 +--
 drivers/gpu/drm/radeon/ni.c| 22 +++
 drivers/gpu/drm/radeon/r100.c  | 18 ++
 drivers/gpu/drm/radeon/r200.c  |  3 +-
 drivers/gpu/drm/radeon/r300.c  | 13 ++--
 drivers/gpu/drm/radeon/r420.c  |  9 +--
 drivers/gpu/drm/radeon/r520.c  |  3 +-
 drivers/gpu/drm/radeon/r600.c  | 21 +++
 drivers/gpu/drm/radeon/r600_cs.c   |  7 +--
 drivers/gpu/drm/radeon/r600_dpm.c  | 71 +++---
 drivers/gpu/drm/radeon/radeon.h|  3 +-
 drivers/gpu/drm/radeon/radeon_atpx_handler.c   |  4 +-
 drivers/gpu/drm/radeon/radeon_audio.c  |  4 +-
 drivers/gpu/drm/radeon/radeon_clocks.c |  2 +-
 drivers/gpu/drm/radeon/radeon_device.c |  8 +--
 drivers/gpu/drm/radeon/radeon_fb.c |  3 +-
 drivers/gpu/drm/radeon/radeon_gem.c|  4 +-
 drivers/gpu/drm/radeon/radeon_test.c   |  6 +-
 drivers/gpu/drm/radeon/rs400.c |  4 +-
 drivers/gpu/drm/radeon/rs690.c |  3 +-
 drivers/gpu/drm/radeon/rv515.c |  9 +--
 drivers/gpu/drm/radeon/si.c| 45 +-
 drivers/gpu/drm/ttm/ttm_bo.c   |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_fence.c  |  6 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c  |  3 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_resource.c   |  4 +-
 71 files changed, 326 insertions(+), 430 deletions(-)

-- 
2.10.0.rc2.1.g053435c

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

[PATCH 1/2] drm: Use pr_cont where appropriate

[Joe Perches]

Using 'printk("\n")' is not preferred anymore and
using printk to continue logging messages now produces
multiple line logging output unless the continuations
use KERN_CONT.

Convert these uses to appropriately use pr_cont or a
single printk where possible.

Miscellanea:

o Use a temporary const char * instead of multiple printks
o Remove trailing space from logging by using a leading space instead

Signed-off-by: Joe Perches <j...@perches.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.c | 70 
 drivers/gpu/drm/radeon/r600_dpm.c   | 71 +
 2 files changed, 73 insertions(+), 68 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.c
index 6ca0333ca4c0..38e9b0d3659a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dpm.c
@@ -31,86 +31,88 @@
 
 void amdgpu_dpm_print_class_info(u32 class, u32 class2)
 {
-   printk("\tui class: ");
+   const char *s;
+
switch (class & ATOM_PPLIB_CLASSIFICATION_UI_MASK) {
case ATOM_PPLIB_CLASSIFICATION_UI_NONE:
default:
-   printk("none\n");
+   s = "none";
break;
case ATOM_PPLIB_CLASSIFICATION_UI_BATTERY:
-   printk("battery\n");
+   s = "battery";
break;
case ATOM_PPLIB_CLASSIFICATION_UI_BALANCED:
-   printk("balanced\n");
+   s = "balanced";
break;
case ATOM_PPLIB_CLASSIFICATION_UI_PERFORMANCE:
-   printk("performance\n");
+   s = "performance";
break;
}
-   printk("\tinternal class: ");
+   printk("\tui class: %s\n", s);
+   printk("\tinternal class:");
if (((class & ~ATOM_PPLIB_CLASSIFICATION_UI_MASK) == 0) &&
(class2 == 0))
-   printk("none");
+   pr_cont(" none");
else {
if (class & ATOM_PPLIB_CLASSIFICATION_BOOT)
-   printk("boot ");
+   pr_cont(" boot");
if (class & ATOM_PPLIB_CLASSIFICATION_THERMAL)
-   printk("thermal ");
+   pr_cont(" thermal");
if (class & ATOM_PPLIB_CLASSIFICATION_LIMITEDPOWERSOURCE)
-   printk("limited_pwr ");
+   pr_cont(" limited_pwr");
if (class & ATOM_PPLIB_CLASSIFICATION_REST)
-   printk("rest ");
+   pr_cont(" rest");
if (class & ATOM_PPLIB_CLASSIFICATION_FORCED)
-   printk("forced ");
+   pr_cont(" forced");
if (class & ATOM_PPLIB_CLASSIFICATION_3DPERFORMANCE)
-   printk("3d_perf ");
+   pr_cont(" 3d_perf");
if (class & ATOM_PPLIB_CLASSIFICATION_OVERDRIVETEMPLATE)
-   printk("ovrdrv ");
+   pr_cont(" ovrdrv");
if (class & ATOM_PPLIB_CLASSIFICATION_UVDSTATE)
-   printk("uvd ");
+   pr_cont(" uvd");
if (class & ATOM_PPLIB_CLASSIFICATION_3DLOW)
-   printk("3d_low ");
+   pr_cont(" 3d_low");
if (class & ATOM_PPLIB_CLASSIFICATION_ACPI)
-   printk("acpi ");
+   pr_cont(" acpi");
if (class & ATOM_PPLIB_CLASSIFICATION_HD2STATE)
-   printk("uvd_hd2 ");
+   pr_cont(" uvd_hd2");
if (class & ATOM_PPLIB_CLASSIFICATION_HDSTATE)
-   printk("uvd_hd ");
+   pr_cont(" uvd_hd");
if (class & ATOM_PPLIB_CLASSIFICATION_SDSTATE)
-   printk("uvd_sd ");
+   pr_cont(" uvd_sd");
if (class2 & ATOM_PPLIB_CLASSIFICATION2_LIMITEDPOWERSOURCE_2)
-   printk("limited_pwr2 ");
+   pr_cont(" limited_pwr2");
if (class2 & ATOM_PPLIB_CLASSIFICATION2_ULV)
-   printk("ulv ");
+   pr_cont(" ulv");
if (class2 & ATOM_PPLIB_CLASSIFICATION2_MVC)
-   printk("uvd_mvc ");
+   pr_cont(" uvd_mvc");
}
- 

Re: [PATCH 00/35] treewide trivial patches converting pr_warning to pr_warn

[Joe Perches]

On Thu, 2017-02-23 at 17:41 +, Emil Velikov wrote:
> On 23 February 2017 at 17:18, Joe Perches <j...@perches.com> wrote:
> > On Thu, 2017-02-23 at 09:28 -0600, Rob Herring wrote:
> > > On Fri, Feb 17, 2017 at 1:11 AM, Joe Perches <j...@perches.com> wrote:
> > > > There are ~4300 uses of pr_warn and ~250 uses of the older
> > > > pr_warning in the kernel source tree.
> > > > 
> > > > Make the use of pr_warn consistent across all kernel files.
> > > > 
> > > > This excludes all files in tools/ as there is a separate
> > > > define pr_warning for that directory tree and pr_warn is
> > > > not used in tools/.
> > > > 
> > > > Done with 'sed s/\bpr_warning\b/pr_warn/' and some emacsing.
> > 
> > []
> > > Where's the removal of pr_warning so we don't have more sneak in?
> > 
> > After all of these actually get applied,
> > and maybe a cycle or two later, one would
> > get sent.
> > 
> 
> By which point you'll get a few reincarnation of it. So you'll have to
> do the same exercise again :-(

Maybe to one or two files.  Not a big deal.

> I guess the question is - are you expecting to get the series merged
> all together/via one tree ?

No.  The only person that could do that effectively is Linus.

> If not, your plan is perfectly reasonable.

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

Re: [PATCH 00/35] treewide trivial patches converting pr_warning to pr_warn

[Joe Perches]

On Thu, 2017-02-23 at 09:28 -0600, Rob Herring wrote:
> On Fri, Feb 17, 2017 at 1:11 AM, Joe Perches <j...@perches.com> wrote:
> > There are ~4300 uses of pr_warn and ~250 uses of the older
> > pr_warning in the kernel source tree.
> > 
> > Make the use of pr_warn consistent across all kernel files.
> > 
> > This excludes all files in tools/ as there is a separate
> > define pr_warning for that directory tree and pr_warn is
> > not used in tools/.
> > 
> > Done with 'sed s/\bpr_warning\b/pr_warn/' and some emacsing.
[]
> Where's the removal of pr_warning so we don't have more sneak in?

After all of these actually get applied,
and maybe a cycle or two later, one would
get sent.

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

[PATCH 17/35] drivers/gpu: Convert remaining uses of pr_warning to pr_warn

[Joe Perches]

To enable eventual removal of pr_warning

This makes pr_warn use consistent for drivers/gpu

Prior to this patch, there were 15 uses of pr_warning and
20 uses of pr_warn in drivers/gpu

Signed-off-by: Joe Perches <j...@perches.com>
---
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c |  2 +-
 drivers/gpu/drm/amd/powerplay/inc/pp_debug.h |  2 +-
 drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.c  |  4 ++--
 drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.c   | 14 +++---
 drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smc.c |  4 ++--
 drivers/gpu/drm/amd/powerplay/smumgr/tonga_smc.c |  4 ++--
 6 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
index b1de9e8ccdbc..83266408634e 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
@@ -1535,7 +1535,7 @@ static int smu7_get_evv_voltages(struct pp_hwmgr *hwmgr)
if (vddc >= 2000 || vddc == 0)
return -EINVAL;
} else {
-   pr_warning("failed to retrieving EVV 
voltage!\n");
+   pr_warn("failed to retrieving EVV 
voltage!\n");
continue;
}
 
diff --git a/drivers/gpu/drm/amd/powerplay/inc/pp_debug.h 
b/drivers/gpu/drm/amd/powerplay/inc/pp_debug.h
index 072880130cfb..f3f9ebb631a5 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/pp_debug.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/pp_debug.h
@@ -37,7 +37,7 @@
 #define PP_ASSERT_WITH_CODE(cond, msg, code)   \
do {\
if (!(cond)) {  \
-   pr_warning("%s\n", msg);\
+   pr_warn("%s\n", msg);   \
code;   \
}   \
} while (0)
diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.c 
b/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.c
index 0f7a77b7312e..5450f5ef8e89 100644
--- a/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.c
+++ b/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.c
@@ -2131,7 +2131,7 @@ uint32_t fiji_get_offsetof(uint32_t type, uint32_t member)
return offsetof(SMU73_Discrete_DpmTable, 
LowSclkInterruptThreshold);
}
}
-   pr_warning("can't get the offset of type %x member %x\n", type, member);
+   pr_warn("can't get the offset of type %x member %x\n", type, member);
return 0;
 }
 
@@ -2156,7 +2156,7 @@ uint32_t fiji_get_mac_definition(uint32_t value)
return SMU73_MAX_LEVELS_MVDD;
}
 
-   pr_warning("can't get the mac of %x\n", value);
+   pr_warn("can't get the mac of %x\n", value);
return 0;
 }
 
diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.c 
b/drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.c
index ad82161df831..51adf04ab4b3 100644
--- a/drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.c
+++ b/drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.c
@@ -122,7 +122,7 @@ static void iceland_initialize_power_tune_defaults(struct 
pp_hwmgr *hwmgr)
break;
default:
smu_data->power_tune_defaults = _iceland;
-   pr_warning("Unknown V.I. Device ID.\n");
+   pr_warn("Unknown V.I. Device ID.\n");
break;
}
return;
@@ -378,7 +378,7 @@ static int iceland_get_std_voltage_value_sidd(struct 
pp_hwmgr *hwmgr,
return -EINVAL);
 
if (NULL == hwmgr->dyn_state.cac_leakage_table) {
-   pr_warning("CAC Leakage Table does not exist, using vddc.\n");
+   pr_warn("CAC Leakage Table does not exist, using vddc.\n");
return 0;
}
 
@@ -394,7 +394,7 @@ static int iceland_get_std_voltage_value_sidd(struct 
pp_hwmgr *hwmgr,
*lo = 
hwmgr->dyn_state.cac_leakage_table->entries[v_index].Vddc * VOLTAGE_SCALE;
*hi = 
(uint16_t)(hwmgr->dyn_state.cac_leakage_table->entries[v_index].Leakage * 
VOLTAGE_SCALE);
} else {
-   pr_warning("Index from SCLK/VDDC Dependency 
Table exceeds the CAC Leakage Table index, using maximum index from CAC 
table.\n");
+   pr_warn("Index from SCLK/VDDC Dependency Table 
exceeds the CAC Leakage Table index, using maximum index from CAC table.\n");
*lo = 
hwmgr->dyn_state.cac_leakage_table->entries[hwmgr->dyn_state.cac_l

[PATCH 00/35] treewide trivial patches converting pr_warning to pr_warn

[Joe Perches]

There are ~4300 uses of pr_warn and ~250 uses of the older
pr_warning in the kernel source tree.

Make the use of pr_warn consistent across all kernel files.

This excludes all files in tools/ as there is a separate
define pr_warning for that directory tree and pr_warn is
not used in tools/.

Done with 'sed s/\bpr_warning\b/pr_warn/' and some emacsing.

Miscellanea:

o Coalesce formats and realign arguments

Some files not compiled - no cross-compilers

Joe Perches (35):
  alpha: Convert remaining uses of pr_warning to pr_warn
  ARM: ep93xx: Convert remaining uses of pr_warning to pr_warn
  arm64: Convert remaining uses of pr_warning to pr_warn
  arch/blackfin: Convert remaining uses of pr_warning to pr_warn
  ia64: Convert remaining use of pr_warning to pr_warn
  powerpc: Convert remaining uses of pr_warning to pr_warn
  sh: Convert remaining uses of pr_warning to pr_warn
  sparc: Convert remaining use of pr_warning to pr_warn
  x86: Convert remaining uses of pr_warning to pr_warn
  drivers/acpi: Convert remaining uses of pr_warning to pr_warn
  block/drbd: Convert remaining uses of pr_warning to pr_warn
  gdrom: Convert remaining uses of pr_warning to pr_warn
  drivers/char: Convert remaining use of pr_warning to pr_warn
  clocksource: Convert remaining use of pr_warning to pr_warn
  drivers/crypto: Convert remaining uses of pr_warning to pr_warn
  fmc: Convert remaining use of pr_warning to pr_warn
  drivers/gpu: Convert remaining uses of pr_warning to pr_warn
  drivers/ide: Convert remaining uses of pr_warning to pr_warn
  drivers/input: Convert remaining uses of pr_warning to pr_warn
  drivers/isdn: Convert remaining uses of pr_warning to pr_warn
  drivers/macintosh: Convert remaining uses of pr_warning to pr_warn
  drivers/media: Convert remaining use of pr_warning to pr_warn
  drivers/mfd: Convert remaining uses of pr_warning to pr_warn
  drivers/mtd: Convert remaining uses of pr_warning to pr_warn
  drivers/of: Convert remaining uses of pr_warning to pr_warn
  drivers/oprofile: Convert remaining uses of pr_warning to pr_warn
  drivers/platform: Convert remaining uses of pr_warning to pr_warn
  drivers/rapidio: Convert remaining use of pr_warning to pr_warn
  drivers/scsi: Convert remaining use of pr_warning to pr_warn
  drivers/sh: Convert remaining use of pr_warning to pr_warn
  drivers/tty: Convert remaining uses of pr_warning to pr_warn
  drivers/video: Convert remaining uses of pr_warning to pr_warn
  kernel/trace: Convert remaining uses of pr_warning to pr_warn
  lib: Convert remaining uses of pr_warning to pr_warn
  sound/soc: Convert remaining uses of pr_warning to pr_warn

 arch/alpha/kernel/perf_event.c |  4 +-
 arch/arm/mach-ep93xx/core.c|  4 +-
 arch/arm64/include/asm/syscall.h   |  8 ++--
 arch/arm64/kernel/hw_breakpoint.c  |  8 ++--
 arch/arm64/kernel/smp.c|  4 +-
 arch/blackfin/kernel/nmi.c |  2 +-
 arch/blackfin/kernel/ptrace.c  |  2 +-
 arch/blackfin/mach-bf533/boards/stamp.c|  2 +-
 arch/blackfin/mach-bf537/boards/cm_bf537e.c|  2 +-
 arch/blackfin/mach-bf537/boards/cm_bf537u.c|  2 +-
 arch/blackfin/mach-bf537/boards/stamp.c|  2 +-
 arch/blackfin/mach-bf537/boards/tcm_bf537.c|  2 +-
 arch/blackfin/mach-bf561/boards/cm_bf561.c |  2 +-
 arch/blackfin/mach-bf561/boards/ezkit.c|  2 +-
 arch/blackfin/mm/isram-driver.c|  4 +-
 arch/ia64/kernel/setup.c   |  6 +--
 arch/powerpc/kernel/pci-common.c   |  4 +-
 arch/powerpc/mm/init_64.c  |  5 +--
 arch/powerpc/mm/mem.c  |  3 +-
 arch/powerpc/platforms/512x/mpc512x_shared.c   |  4 +-
 arch/powerpc/platforms/85xx/socrates_fpga_pic.c|  7 ++--
 arch/powerpc/platforms/86xx/mpc86xx_hpcn.c |  2 +-
 arch/powerpc/platforms/pasemi/dma_lib.c|  4 +-
 arch/powerpc/platforms/powernv/opal.c  |  8 ++--
 arch/powerpc/platforms/powernv/pci-ioda.c  | 10 ++---
 arch/powerpc/platforms/ps3/device-init.c   | 14 +++
 arch/powerpc/platforms/ps3/mm.c|  4 +-
 arch/powerpc/platforms/ps3/os-area.c   |  2 +-
 arch/powerpc/platforms/pseries/iommu.c |  8 ++--
 arch/powerpc/platforms/pseries/setup.c |  4 +-
 arch/powerpc/sysdev/fsl_pci.c  |  9 ++---
 arch/powerpc/sysdev/mpic.c | 10 ++---
 arch/powerpc/sysdev/xics/icp-native.c  | 10 ++---
 arch/powerpc/sysdev/xics/ics-opal.c|  4 +-
 arch/powerpc/sysdev/xics/ics-rtas.c|  4 +-
 arch/powerpc/sysdev/xics/xics-common.c |  8 ++--
 arch/sh/boards/mach-sdk7786/nmi.c  |  2 +-
 arch/sh/drivers/pci/fixups-sdk7786.c   |  2 +-
 arch/sh/kernel/io_trapped.c