Re: [PATCH] tracing/treewide: Remove second parameter of __assign_str()

2024-05-17 Thread Guenter Roeck

On 5/17/24 11:00, Guenter Roeck wrote:

On 5/17/24 10:48, Steven Rostedt wrote:

On Fri, 17 May 2024 10:36:37 -0700
Guenter Roeck  wrote:


Building csky:allmodconfig (and others) ... failed
--
Error log:
In file included from include/trace/trace_events.h:419,
  from include/trace/define_trace.h:102,
  from drivers/cxl/core/trace.h:737,
  from drivers/cxl/core/trace.c:8:
drivers/cxl/core/./trace.h:383:1: error: macro "__assign_str" passed 2 
arguments, but takes just 1

This is with the patch applied on top of v6.9-8410-gff2632d7d08e.
So far that seems to be the only build failure.
Introduced with commit 6aec00139d3a8 ("cxl/core: Add region info to
cxl_general_media and cxl_dram events"). Guess we'll see more of those
towards the end of the commit window.


Looks like I made this patch just before this commit was pulled into
Linus's tree.

Which is why I'll apply and rerun the above again probably on Tuesday of
next week against Linus's latest.

This patch made it through both an allyesconfig and an allmodconfig, but on
the commit I had applied it to, which was:

   1b294a1f3561 ("Merge tag 'net-next-6.10' of 
git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next")

I'll be compiling those two builds after I update it then.



I am currently repeating my test builds with the above errors fixed.
That should take a couple of hours. I'll let you know how it goes.



There are no more build failures caused by this patch after fixing the above
errors.

Tested-by: Guenter Roeck 

Guenter



[RFC PATCH 4/4] drm/msm: switch msm_kms to use msm_iommu_disp_new()

2024-05-17 Thread Abhinav Kumar
Switch msm_kms to use msm_iommu_disp_new() so that the newly
registered fault handler will kick-in during any mmu faults.

Signed-off-by: Abhinav Kumar 
---
 drivers/gpu/drm/msm/msm_kms.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/msm_kms.c b/drivers/gpu/drm/msm/msm_kms.c
index 62c8e6163e81..1859efbbff1d 100644
--- a/drivers/gpu/drm/msm/msm_kms.c
+++ b/drivers/gpu/drm/msm/msm_kms.c
@@ -181,7 +181,7 @@ struct msm_gem_address_space *msm_kms_init_aspace(struct 
drm_device *dev)
else
iommu_dev = mdss_dev;
 
-   mmu = msm_iommu_new(iommu_dev, 0);
+   mmu = msm_iommu_disp_new(iommu_dev, 0);
if (IS_ERR(mmu))
return ERR_CAST(mmu);
 
-- 
2.44.0



[RFC PATCH 3/4] drm/msm/iommu: introduce msm_iommu_disp_new() for msm_kms

2024-05-17 Thread Abhinav Kumar
Introduce a new API msm_iommu_disp_new() for display use-cases.

Signed-off-by: Abhinav Kumar 
---
 drivers/gpu/drm/msm/msm_iommu.c | 28 
 drivers/gpu/drm/msm/msm_mmu.h   |  1 +
 2 files changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
index a79cd18bc4c9..3d5c1bb4c013 100644
--- a/drivers/gpu/drm/msm/msm_iommu.c
+++ b/drivers/gpu/drm/msm/msm_iommu.c
@@ -343,6 +343,19 @@ static int msm_gpu_fault_handler(struct iommu_domain 
*domain, struct device *dev
return 0;
 }
 
+static int msm_disp_fault_handler(struct iommu_domain *domain, struct device 
*dev,
+ unsigned long iova, int flags, void *arg)
+{
+   struct msm_iommu *iommu = arg;
+
+   if (iommu->base.handler)
+   return iommu->base.handler(iommu->base.arg, iova, flags, NULL);
+
+   pr_warn_ratelimited("*** fault: iova=%16lx, flags=%d\n", iova, flags);
+
+   return 0;
+}
+
 static void msm_iommu_resume_translation(struct msm_mmu *mmu)
 {
struct adreno_smmu_priv *adreno_smmu = dev_get_drvdata(mmu->dev);
@@ -434,6 +447,21 @@ struct msm_mmu *msm_iommu_new(struct device *dev, unsigned 
long quirks)
return >base;
 }
 
+struct msm_mmu *msm_iommu_disp_new(struct device *dev, unsigned long quirks)
+{
+   struct msm_iommu *iommu;
+   struct msm_mmu *mmu;
+
+   mmu = msm_iommu_new(dev, quirks);
+   if (IS_ERR_OR_NULL(mmu))
+   return mmu;
+
+   iommu = to_msm_iommu(mmu);
+   iommu_set_fault_handler(iommu->domain, msm_disp_fault_handler, iommu);
+
+   return mmu;
+}
+
 struct msm_mmu *msm_iommu_gpu_new(struct device *dev, struct msm_gpu *gpu, 
unsigned long quirks)
 {
struct adreno_smmu_priv *adreno_smmu = dev_get_drvdata(dev);
diff --git a/drivers/gpu/drm/msm/msm_mmu.h b/drivers/gpu/drm/msm/msm_mmu.h
index 88af4f490881..730458d08d6b 100644
--- a/drivers/gpu/drm/msm/msm_mmu.h
+++ b/drivers/gpu/drm/msm/msm_mmu.h
@@ -42,6 +42,7 @@ static inline void msm_mmu_init(struct msm_mmu *mmu, struct 
device *dev,
 
 struct msm_mmu *msm_iommu_new(struct device *dev, unsigned long quirks);
 struct msm_mmu *msm_iommu_gpu_new(struct device *dev, struct msm_gpu *gpu, 
unsigned long quirks);
+struct msm_mmu *msm_iommu_disp_new(struct device *dev, unsigned long quirks);
 
 static inline void msm_mmu_set_fault_handler(struct msm_mmu *mmu, void *arg,
int (*handler)(void *arg, unsigned long iova, int flags, void 
*data))
-- 
2.44.0



[RFC PATCH 2/4] drm/msm/iommu: rename msm_fault_handler to msm_gpu_fault_handler

2024-05-17 Thread Abhinav Kumar
In preparation of registering a separate fault handler for
display, lets rename the existing msm_fault_handler to
msm_gpu_fault_handler.

Signed-off-by: Abhinav Kumar 
---
 drivers/gpu/drm/msm/msm_iommu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
index d5512037c38b..a79cd18bc4c9 100644
--- a/drivers/gpu/drm/msm/msm_iommu.c
+++ b/drivers/gpu/drm/msm/msm_iommu.c
@@ -243,7 +243,7 @@ static const struct iommu_flush_ops tlb_ops = {
.tlb_add_page = msm_iommu_tlb_add_page,
 };
 
-static int msm_fault_handler(struct iommu_domain *domain, struct device *dev,
+static int msm_gpu_fault_handler(struct iommu_domain *domain, struct device 
*dev,
unsigned long iova, int flags, void *arg);
 
 struct msm_mmu *msm_iommu_pagetable_create(struct msm_mmu *parent)
@@ -319,7 +319,7 @@ struct msm_mmu *msm_iommu_pagetable_create(struct msm_mmu 
*parent)
return >base;
 }
 
-static int msm_fault_handler(struct iommu_domain *domain, struct device *dev,
+static int msm_gpu_fault_handler(struct iommu_domain *domain, struct device 
*dev,
unsigned long iova, int flags, void *arg)
 {
struct msm_iommu *iommu = arg;
@@ -445,7 +445,7 @@ struct msm_mmu *msm_iommu_gpu_new(struct device *dev, 
struct msm_gpu *gpu, unsig
return mmu;
 
iommu = to_msm_iommu(mmu);
-   iommu_set_fault_handler(iommu->domain, msm_fault_handler, iommu);
+   iommu_set_fault_handler(iommu->domain, msm_gpu_fault_handler, iommu);
 
/* Enable stall on iommu fault: */
if (adreno_smmu->set_stall)
-- 
2.44.0



[RFC PATCH 0/4] drm/msm: add a display mmu fault handler

2024-05-17 Thread Abhinav Kumar
To debug display mmu faults, this series introduces a display fault
handler similar to the gpu one.

This is only compile tested at the moment, till a suitable method
to trigger the fault is found and see if this handler does the needful
on the device.

Abhinav Kumar (4):
  drm/msm: register a fault handler for display mmu faults
  drm/msm/iommu: rename msm_fault_handler to msm_gpu_fault_handler
  drm/msm/iommu: introduce msm_iommu_disp_new() for msm_kms
  drm/msm: switch msm_kms to use msm_iommu_disp_new()

 drivers/gpu/drm/msm/msm_iommu.c | 34 ++---
 drivers/gpu/drm/msm/msm_kms.c   | 27 +-
 drivers/gpu/drm/msm/msm_mmu.h   |  1 +
 3 files changed, 58 insertions(+), 4 deletions(-)

-- 
2.44.0



[RFC PATCH 1/4] drm/msm: register a fault handler for display mmu faults

2024-05-17 Thread Abhinav Kumar
In preparation to register a iommu fault handler for display
related modules, register a fault handler for the backing
mmu object of msm_kms.

Currently, the fault handler only captures the display snapshot
but we can expand this later if more information needs to be
added to debug display mmu faults.

Signed-off-by: Abhinav Kumar 
---
 drivers/gpu/drm/msm/msm_kms.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/msm/msm_kms.c b/drivers/gpu/drm/msm/msm_kms.c
index af6a6fcb1173..62c8e6163e81 100644
--- a/drivers/gpu/drm/msm/msm_kms.c
+++ b/drivers/gpu/drm/msm/msm_kms.c
@@ -200,6 +200,28 @@ struct msm_gem_address_space *msm_kms_init_aspace(struct 
drm_device *dev)
return aspace;
 }
 
+static int msm_kms_fault_handler(void *arg, unsigned long iova, int flags, 
void *data)
+{
+   struct msm_kms *kms = arg;
+   struct msm_disp_state *state;
+   int ret;
+
+   ret = mutex_lock_interruptible(>dump_mutex);
+   if (ret)
+   return ret;
+
+   state = msm_disp_snapshot_state_sync(kms);
+
+   mutex_unlock(>dump_mutex);
+
+   if (IS_ERR(state)) {
+   DRM_DEV_ERROR(kms->dev->dev, "failed to capture snapshot\n");
+   return PTR_ERR(state);
+   }
+
+   return 0;
+}
+
 void msm_drm_kms_uninit(struct device *dev)
 {
struct platform_device *pdev = to_platform_device(dev);
@@ -261,6 +283,9 @@ int msm_drm_kms_init(struct device *dev, const struct 
drm_driver *drv)
goto err_msm_uninit;
}
 
+   if (kms->aspace)
+   msm_mmu_set_fault_handler(kms->aspace->mmu, kms, 
msm_kms_fault_handler);
+
drm_helper_move_panel_connectors_to_head(ddev);
 
drm_for_each_crtc(crtc, ddev) {
-- 
2.44.0



[PATCH] drm/msm/adreno: Check for zap node availability

2024-05-17 Thread Rob Clark
From: Rob Clark 

This should allow disabling the zap node via an overlay, for slbounce.

Suggested-by: Nikita Travkin 
Signed-off-by: Rob Clark 
---
 drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c 
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index d9ea15994ae9..a00241e3373b 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -46,7 +46,7 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const 
char *fwname,
}
 
np = of_get_child_by_name(dev->of_node, "zap-shader");
-   if (!np) {
+   if (!np || !of_device_is_available(np)) {
zap_available = false;
return -ENODEV;
}
-- 
2.45.1



Re: [PATCH] tracing/treewide: Remove second parameter of __assign_str()

2024-05-17 Thread Guenter Roeck

On 5/17/24 10:48, Steven Rostedt wrote:

On Fri, 17 May 2024 10:36:37 -0700
Guenter Roeck  wrote:


Building csky:allmodconfig (and others) ... failed
--
Error log:
In file included from include/trace/trace_events.h:419,
  from include/trace/define_trace.h:102,
  from drivers/cxl/core/trace.h:737,
  from drivers/cxl/core/trace.c:8:
drivers/cxl/core/./trace.h:383:1: error: macro "__assign_str" passed 2 
arguments, but takes just 1

This is with the patch applied on top of v6.9-8410-gff2632d7d08e.
So far that seems to be the only build failure.
Introduced with commit 6aec00139d3a8 ("cxl/core: Add region info to
cxl_general_media and cxl_dram events"). Guess we'll see more of those
towards the end of the commit window.


Looks like I made this patch just before this commit was pulled into
Linus's tree.

Which is why I'll apply and rerun the above again probably on Tuesday of
next week against Linus's latest.

This patch made it through both an allyesconfig and an allmodconfig, but on
the commit I had applied it to, which was:

   1b294a1f3561 ("Merge tag 'net-next-6.10' of 
git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next")

I'll be compiling those two builds after I update it then.



I am currently repeating my test builds with the above errors fixed.
That should take a couple of hours. I'll let you know how it goes.

Guenter



Re: [PATCH] tracing/treewide: Remove second parameter of __assign_str()

2024-05-17 Thread Guenter Roeck
On Thu, May 16, 2024 at 01:34:54PM -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" 
> 
> [
>This is a treewide change. I will likely re-create this patch again in
>the second week of the merge window of v6.10 and submit it then. Hoping
>to keep the conflicts that it will cause to a minimum.
> ]
> 
> With the rework of how the __string() handles dynamic strings where it
> saves off the source string in field in the helper structure[1], the
> assignment of that value to the trace event field is stored in the helper
> value and does not need to be passed in again.
> 
> This means that with:
> 
>   __string(field, mystring)
> 
> Which use to be assigned with __assign_str(field, mystring), no longer
> needs the second parameter and it is unused. With this, __assign_str()
> will now only get a single parameter.
> 
> There's over 700 users of __assign_str() and because coccinelle does not
> handle the TRACE_EVENT() macro I ended up using the following sed script:
> 
>   git grep -l __assign_str | while read a ; do
>   sed -e 's/\(__assign_str([^,]*[^ ,]\) *,[^;]*/\1)/' $a > /tmp/test-file;
>   mv /tmp/test-file $a;
>   done
> 
> I then searched for __assign_str() that did not end with ';' as those
> were multi line assignments that the sed script above would fail to catch.
> 

Building csky:allmodconfig (and others) ... failed
--
Error log:
In file included from include/trace/trace_events.h:419,
 from include/trace/define_trace.h:102,
 from drivers/cxl/core/trace.h:737,
 from drivers/cxl/core/trace.c:8:
drivers/cxl/core/./trace.h:383:1: error: macro "__assign_str" passed 2 
arguments, but takes just 1

This is with the patch applied on top of v6.9-8410-gff2632d7d08e.
So far that seems to be the only build failure.
Introduced with commit 6aec00139d3a8 ("cxl/core: Add region info to
cxl_general_media and cxl_dram events"). Guess we'll see more of those
towards the end of the commit window.

Guenter


Re: [PATCH] tracing/treewide: Remove second parameter of __assign_str()

2024-05-17 Thread Darrick J. Wong
On Thu, May 16, 2024 at 01:34:54PM -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" 
> 
> [
>This is a treewide change. I will likely re-create this patch again in
>the second week of the merge window of v6.10 and submit it then. Hoping
>to keep the conflicts that it will cause to a minimum.
> ]
> 
> With the rework of how the __string() handles dynamic strings where it
> saves off the source string in field in the helper structure[1], the
> assignment of that value to the trace event field is stored in the helper
> value and does not need to be passed in again.
> 
> This means that with:
> 
>   __string(field, mystring)
> 
> Which use to be assigned with __assign_str(field, mystring), no longer
> needs the second parameter and it is unused. With this, __assign_str()
> will now only get a single parameter.
> 
> There's over 700 users of __assign_str() and because coccinelle does not
> handle the TRACE_EVENT() macro I ended up using the following sed script:
> 
>   git grep -l __assign_str | while read a ; do
>   sed -e 's/\(__assign_str([^,]*[^ ,]\) *,[^;]*/\1)/' $a > /tmp/test-file;
>   mv /tmp/test-file $a;
>   done
> 
> I then searched for __assign_str() that did not end with ';' as those
> were multi line assignments that the sed script above would fail to catch.
> 
> Note, the same updates will need to be done for:
> 
>   __assign_str_len()
>   __assign_rel_str()
>   __assign_rel_str_len()
> 
> I tested this with both an allmodconfig and an allyesconfig (build only for 
> both).
> 
> [1] 
> https://lore.kernel.org/linux-trace-kernel/2024011442.634192...@goodmis.org/
> 
> Cc: Masami Hiramatsu 
> Cc: Mathieu Desnoyers 
> Cc: Linus Torvalds 
> Cc: Julia Lawall 
> Signed-off-by: Steven Rostedt (Google) 

/me finds this pretty magical, but such is the way of macros.
Thanks for being much smarter about them than me. :)

Acked-by: Darrick J. Wong# xfs

--D


Re: [PATCH v2] arm64: dts: qcom: msm8998: enable adreno_smmu by default

2024-05-17 Thread Jeffrey Hugo

On 5/15/2024 8:27 AM, Marc Gonzalez wrote:

15 qcom platform DTSI files define an adreno_smmu node.
msm8998 is the only one with adreno_smmu disabled by default.

There's no reason why this SMMU should be disabled by default,
it doesn't need any further configuration.

Bring msm8998 in line with the 14 other platforms.

This fixes GPU init failing with ENODEV:
msm_dpu c901000.display-controller: failed to load adreno gpu
msm_dpu c901000.display-controller: failed to bind 500.gpu (ops a3xx_ops): 
-19

Fixes: 87cd46d68aeac8 ("Configure Adreno GPU and related IOMMU")
Signed-off-by: Marc Gonzalez 
Reviewed-by: Bryan O'Donoghue 


Reviewed-by: Jeffrey Hugo 


Re: [PATCH] tracing/treewide: Remove second parameter of __assign_str()

2024-05-17 Thread Takashi Iwai
On Thu, 16 May 2024 19:34:54 +0200,
Steven Rostedt wrote:
> 
> From: "Steven Rostedt (Google)" 
> 
> [
>This is a treewide change. I will likely re-create this patch again in
>the second week of the merge window of v6.10 and submit it then. Hoping
>to keep the conflicts that it will cause to a minimum.
> ]
> 
> With the rework of how the __string() handles dynamic strings where it
> saves off the source string in field in the helper structure[1], the
> assignment of that value to the trace event field is stored in the helper
> value and does not need to be passed in again.
> 
> This means that with:
> 
>   __string(field, mystring)
> 
> Which use to be assigned with __assign_str(field, mystring), no longer
> needs the second parameter and it is unused. With this, __assign_str()
> will now only get a single parameter.
> 
> There's over 700 users of __assign_str() and because coccinelle does not
> handle the TRACE_EVENT() macro I ended up using the following sed script:
> 
>   git grep -l __assign_str | while read a ; do
>   sed -e 's/\(__assign_str([^,]*[^ ,]\) *,[^;]*/\1)/' $a > /tmp/test-file;
>   mv /tmp/test-file $a;
>   done
> 
> I then searched for __assign_str() that did not end with ';' as those
> were multi line assignments that the sed script above would fail to catch.
> 
> Note, the same updates will need to be done for:
> 
>   __assign_str_len()
>   __assign_rel_str()
>   __assign_rel_str_len()
> 
> I tested this with both an allmodconfig and an allyesconfig (build only for 
> both).
> 
> [1] 
> https://lore.kernel.org/linux-trace-kernel/2024011442.634192...@goodmis.org/
> 
> Cc: Masami Hiramatsu 
> Cc: Mathieu Desnoyers 
> Cc: Linus Torvalds 
> Cc: Julia Lawall 
> Signed-off-by: Steven Rostedt (Google) 

For the sound part
Acked-by: Takashi Iwai 


thanks,

Takashi


Re: [PATCH] tracing/treewide: Remove second parameter of __assign_str()

2024-05-17 Thread Rafael J. Wysocki
On Thu, May 16, 2024 at 7:35 PM Steven Rostedt  wrote:
>
> From: "Steven Rostedt (Google)" 
>
> [
>This is a treewide change. I will likely re-create this patch again in
>the second week of the merge window of v6.10 and submit it then. Hoping
>to keep the conflicts that it will cause to a minimum.
> ]
>
> With the rework of how the __string() handles dynamic strings where it
> saves off the source string in field in the helper structure[1], the
> assignment of that value to the trace event field is stored in the helper
> value and does not need to be passed in again.
>
> This means that with:
>
>   __string(field, mystring)
>
> Which use to be assigned with __assign_str(field, mystring), no longer
> needs the second parameter and it is unused. With this, __assign_str()
> will now only get a single parameter.
>
> There's over 700 users of __assign_str() and because coccinelle does not
> handle the TRACE_EVENT() macro I ended up using the following sed script:
>
>   git grep -l __assign_str | while read a ; do
>   sed -e 's/\(__assign_str([^,]*[^ ,]\) *,[^;]*/\1)/' $a > /tmp/test-file;
>   mv /tmp/test-file $a;
>   done
>
> I then searched for __assign_str() that did not end with ';' as those
> were multi line assignments that the sed script above would fail to catch.
>
> Note, the same updates will need to be done for:
>
>   __assign_str_len()
>   __assign_rel_str()
>   __assign_rel_str_len()
>
> I tested this with both an allmodconfig and an allyesconfig (build only for 
> both).
>
> [1] 
> https://lore.kernel.org/linux-trace-kernel/2024011442.634192...@goodmis.org/
>
> Cc: Masami Hiramatsu 
> Cc: Mathieu Desnoyers 
> Cc: Linus Torvalds 
> Cc: Julia Lawall 
> Signed-off-by: Steven Rostedt (Google) 

Acked-by: Rafael J. Wysocki  # for thermal