[Freedreno] [PATCH 2/2] drm/msm/dpu: Remove _dpu_debugfs_init
From: Sean Paul
Fold it into dpu_debugfs_init.
Cc: Stephen Boyd
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index d77071965431..0a8c334c3a9f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -231,8 +231,9 @@ void *dpu_debugfs_create_regset32(const char *name, umode_t
mode,
regset, &dpu_fops_regset32);
}
-static int _dpu_debugfs_init(struct dpu_kms *dpu_kms, struct drm_minor *minor)
+static int dpu_kms_debugfs_init(struct msm_kms *kms, struct drm_minor *minor)
{
+ struct dpu_kms *dpu_kms = to_dpu_kms(kms);
void *p = dpu_hw_util_get_log_mask_ptr();
struct dentry *entry;
@@ -578,13 +579,6 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
return ret;
}
-#ifdef CONFIG_DEBUG_FS
-static int dpu_kms_debugfs_init(struct msm_kms *kms, struct drm_minor *minor)
-{
- return _dpu_debugfs_init(to_dpu_kms(kms), minor);
-}
-#endif
-
static long dpu_kms_round_pixclk(struct msm_kms *kms, unsigned long rate,
struct drm_encoder *encoder)
{
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 1/2] drm/msm/dpu: Use provided drm_minor to initialize debugfs
From: Sean Paul
Instead of reaching into dev->primary for debugfs_root, use the minor
passed into debugfs_init.
This avoids creating the debug directory under /sys/kernel/debug/ and
instead creates the directory under the correct node in
/sys/kernel/debug/dri//
Reported-by: Stephen Boyd
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 885bf88afa3e..d77071965431 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -231,7 +231,7 @@ void *dpu_debugfs_create_regset32(const char *name, umode_t
mode,
regset, &dpu_fops_regset32);
}
-static int _dpu_debugfs_init(struct dpu_kms *dpu_kms)
+static int _dpu_debugfs_init(struct dpu_kms *dpu_kms, struct drm_minor *minor)
{
void *p = dpu_hw_util_get_log_mask_ptr();
struct dentry *entry;
@@ -239,7 +239,7 @@ static int _dpu_debugfs_init(struct dpu_kms *dpu_kms)
if (!p)
return -EINVAL;
- entry = debugfs_create_dir("debug",
dpu_kms->dev->primary->debugfs_root);
+ entry = debugfs_create_dir("debug", minor->debugfs_root);
if (IS_ERR_OR_NULL(entry))
return -ENODEV;
@@ -581,7 +581,7 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
#ifdef CONFIG_DEBUG_FS
static int dpu_kms_debugfs_init(struct msm_kms *kms, struct drm_minor *minor)
{
- return _dpu_debugfs_init(to_dpu_kms(kms));
+ return _dpu_debugfs_init(to_dpu_kms(kms), minor);
}
#endif
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH] iommu: arm-smmu: Set SCTLR.HUPCF bit
On Mon, Nov 26, 2018 at 11:31 AM Will Deacon wrote: > > Hi Rob, > > On Tue, Nov 13, 2018 at 08:12:35AM -0500, Rob Clark wrote: > > On Tue, Nov 13, 2018 at 1:32 AM Will Deacon wrote: > > > On Fri, Nov 09, 2018 at 01:01:55PM -0500, Rob Clark wrote: > > > > On Mon, Oct 29, 2018 at 3:09 PM Will Deacon wrote: > > > > > On Thu, Sep 27, 2018 at 06:46:07PM -0400, Rob Clark wrote: > > > > > > We seem to need to set either this or CFCFG (stall), otherwise gpu > > > > > > faults trigger problems with other in-flight transactions from the > > > > > > GPU causing CP errors, etc. > > > > > > > > > > > > In the ARM SMMU spec, the 'Hit under previous context fault' bit is > > > > > > described as: > > > > > > > > > > > > '0' - Stall or terminate subsequent transactions in the presence > > > > > >of an outstanding context fault > > > > > > '1' - Process all subsequent transactions independently of any > > > > > >outstanding context fault. > > > > > > > > > > > > Since we don't enable CFCFG (stall) the behavior of terminating > > > > > > other transactions makes sense. And is probably not what we want > > > > > > (and definately not what we want for GPU). > > > > > > > > > > > > Signed-off-by: Rob Clark > > > > > > --- > > > > > > So I hit this issue a long time back on 820 (msm8996) and at the > > > > > > time I solved it with a patch that enabled CFCFG. And it resurfaced > > > > > > more recently on sdm845. But at the time CFCFG was rejected, iirc > > > > > > because of concern that it would cause problems on other non-qcom > > > > > > arm smmu implementations. And I think I forgot to send this version > > > > > > of the solution. > > > > > > > > > > > > If enabling HUPCF is anticipated to cause problems on other ARM > > > > > > SMMU implementations, I think I can come up with a variant of this > > > > > > patch which conditionally enables it for snapdragon. > > > > > > > > > > > > Either way, I'd really like to get some variant of this fix merged > > > > > > (and probably it would be a good idea for stable kernel branches > > > > > > too), since current behaviour with the GPU means faults turn into > > > > > > a fantastic cascade of fail. > > > > > > > > > > Can you describe how this fantastic cascade of fail improves with this > > > > > patch, please? If you're getting context faults then something has > > > > > already > > > > > gone horribly wrong, so I'm trying to work out how this improves > > > > > things. > > > > > > > > > > > > > There are plenty of cases where getting iommu faults with a GPU is > > > > "normal", or at least not something the kernel or even GL driver can > > > > control. > > > > > > Such as? All the mainline driver does is print a diagnostic and clear the > > > fault, which doesn't seem generally useful. > > > > it is useful to debug the fault ;-) > > > > Although eventually we'll want to be able to do more than that, like > > have the fault trigger bringing in pages of a mmap'd file and that > > sort of thing. > > Right, and feels very strange to me if we have this bit set because it > means that your fault is no longer synchronous and therefore diverges > from the fault model that you get from the CPU, where you certainly > wouldn't expect stores appearing in the program after a faulting load to > be visible in memory. However, thinking harder about it, I suppose we're > already in a situation where the translations are handled out of order > in the absence of barriers, so maybe it's not the end of the world. > > Could you dump the FSR value that you see in the fault handler, please? > From my reading of the architecture spec, as long as clear all of the > fault bits in the irq handler, then your machine shouldn't die like it > does with HUPCFG=CFCFG=0.. Getting back to this after realizing I lost SCTLR.HUPCF patch that I was carrying locally when rebasing. Here is an example dump (w/ FSR) of what happens: [May24 14:33] arm-smmu 504.iommu: Unhandled context fault: fsr=0x402, iova=0x7ffe35c0, fsynr=0x1, cb=1 [ +0.34] adreno 500.gpu: CP illegal instruction error [ +0.36] adreno 500.gpu: CP illegal instruction error [ +0.17] adreno 500.gpu: CP illegal instruction error [ +0.17] adreno 500.gpu: CP illegal instruction error [ +0.15] adreno 500.gpu: CP illegal instruction error [ +0.16] adreno 500.gpu: CP illegal instruction error [ +0.76] adreno 500.gpu: CP illegal instruction error [ +0.15] adreno 500.gpu: CP illegal instruction error [ +0.16] adreno 500.gpu: CP illegal instruction error [ +0.15] adreno 500.gpu: CP illegal instruction error [ +0.047100] adreno 500.gpu: [drm:a6xx_irq] *ERROR* gpu fault ring 0 fence 8 status 0085 rb 0047/0047 ib1 01CC7000/ ib2 01CC5000/ [ +0.000106] msm ae0.mdss: [drm:recover_worker] *ERROR* A630: hangcheck recover! [ +0.000380] msm ae0.mdss: [drm:recover_worker] *ERROR* A630: offending task: d:flus
Re: [Freedreno] [PATCH 1/2] drm/msm/dpu: Use provided drm_minor to initialize debugfs
On 2019-05-24 10:32, Sean Paul wrote:
From: Sean Paul
Instead of reaching into dev->primary for debugfs_root, use the minor
passed into debugfs_init.
This avoids creating the debug directory under /sys/kernel/debug/ and
instead creates the directory under the correct node in
/sys/kernel/debug/dri//
Reported-by: Stephen Boyd
Signed-off-by: Sean Paul
Reviewed-by: Abhinav Kumar
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 885bf88afa3e..d77071965431 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -231,7 +231,7 @@ void *dpu_debugfs_create_regset32(const char
*name, umode_t mode,
regset, &dpu_fops_regset32);
}
-static int _dpu_debugfs_init(struct dpu_kms *dpu_kms)
+static int _dpu_debugfs_init(struct dpu_kms *dpu_kms, struct drm_minor
*minor)
{
void *p = dpu_hw_util_get_log_mask_ptr();
struct dentry *entry;
@@ -239,7 +239,7 @@ static int _dpu_debugfs_init(struct dpu_kms
*dpu_kms)
if (!p)
return -EINVAL;
- entry = debugfs_create_dir("debug",
dpu_kms->dev->primary->debugfs_root);
+ entry = debugfs_create_dir("debug", minor->debugfs_root);
if (IS_ERR_OR_NULL(entry))
return -ENODEV;
@@ -581,7 +581,7 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms
*dpu_kms)
#ifdef CONFIG_DEBUG_FS
static int dpu_kms_debugfs_init(struct msm_kms *kms, struct drm_minor
*minor)
{
- return _dpu_debugfs_init(to_dpu_kms(kms));
+ return _dpu_debugfs_init(to_dpu_kms(kms), minor);
}
#endif
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 2/2] drm/msm/dpu: Remove _dpu_debugfs_init
On 2019-05-24 10:32, Sean Paul wrote:
From: Sean Paul
Fold it into dpu_debugfs_init.
Cc: Stephen Boyd
Signed-off-by: Sean Paul
Reviewed-by: Abhinav Kumar
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index d77071965431..0a8c334c3a9f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -231,8 +231,9 @@ void *dpu_debugfs_create_regset32(const char
*name, umode_t mode,
regset, &dpu_fops_regset32);
}
-static int _dpu_debugfs_init(struct dpu_kms *dpu_kms, struct drm_minor
*minor)
+static int dpu_kms_debugfs_init(struct msm_kms *kms, struct drm_minor
*minor)
{
+ struct dpu_kms *dpu_kms = to_dpu_kms(kms);
void *p = dpu_hw_util_get_log_mask_ptr();
struct dentry *entry;
@@ -578,13 +579,6 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms
*dpu_kms)
return ret;
}
-#ifdef CONFIG_DEBUG_FS
-static int dpu_kms_debugfs_init(struct msm_kms *kms, struct drm_minor
*minor)
-{
- return _dpu_debugfs_init(to_dpu_kms(kms), minor);
-}
-#endif
-
static long dpu_kms_round_pixclk(struct msm_kms *kms, unsigned long
rate,
struct drm_encoder *encoder)
{
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH] drm/msm: Re-order uninit function to work during probe defer
From: Sean Paul
If bind fails, we can call msm_drm_uninit before kms elements have been
created. In this case, drm_atomic_helper_shutdown will fail since there
are no drm objects. Only call drm unregistration and shutdown if drm is
registered.
Also while we're in here move the workqueue destruction to below
component_unbind since components could be actively using the wq during
uninit or in their unbind routine.
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/msm_drv.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 31deb87abfc6..9f16a995ed42 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -259,13 +259,24 @@ static int msm_drm_uninit(struct device *dev)
struct msm_mdss *mdss = priv->mdss;
int i;
+ /*
+* Shutdown the hw if we're far enough along where things might be on.
+* If we run this too early, we'll end up panicking in any variety of
+* places. Since we don't register the drm device until late in
+* msm_drm_init, drm_dev->registered is used as an indicator that the
+* shutdown will be successful.
+*/
+ if (ddev->registered) {
+ drm_dev_unregister(ddev);
+ drm_atomic_helper_shutdown(ddev);
+ }
+
/* We must cancel and cleanup any pending vblank enable/disable
* work before drm_irq_uninstall() to avoid work re-enabling an
* irq after uninstall has disabled it.
*/
flush_workqueue(priv->wq);
- destroy_workqueue(priv->wq);
/* clean up event worker threads */
for (i = 0; i < priv->num_crtcs; i++) {
@@ -279,8 +290,6 @@ static int msm_drm_uninit(struct device *dev)
drm_kms_helper_poll_fini(ddev);
- drm_dev_unregister(ddev);
-
msm_perf_debugfs_cleanup(priv);
msm_rd_debugfs_cleanup(priv);
@@ -288,7 +297,7 @@ static int msm_drm_uninit(struct device *dev)
if (fbdev && priv->fbdev)
msm_fbdev_free(ddev);
#endif
- drm_atomic_helper_shutdown(ddev);
+
drm_mode_config_cleanup(ddev);
pm_runtime_get_sync(dev);
@@ -313,6 +322,7 @@ static int msm_drm_uninit(struct device *dev)
ddev->dev_private = NULL;
drm_dev_put(ddev);
+ destroy_workqueue(priv->wq);
kfree(priv);
return 0;
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 1/2] drm/msm/dpu: Use provided drm_minor to initialize debugfs
Quoting Sean Paul (2019-05-24 10:32:18) > From: Sean Paul > > Instead of reaching into dev->primary for debugfs_root, use the minor > passed into debugfs_init. > > This avoids creating the debug directory under /sys/kernel/debug/ and > instead creates the directory under the correct node in > /sys/kernel/debug/dri// So does this make it become /sys/kernel/debug/dri//debug? I wonder why it can't all be created under /sys/kernel/debug/dri/ and then avoid the extra "debug" directory that isn't adding any value? ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 1/2] drm/msm/dpu: Use provided drm_minor to initialize debugfs
On Fri, May 24, 2019 at 1:43 PM Stephen Boyd wrote: > > Quoting Sean Paul (2019-05-24 10:32:18) > > From: Sean Paul > > > > Instead of reaching into dev->primary for debugfs_root, use the minor > > passed into debugfs_init. > > > > This avoids creating the debug directory under /sys/kernel/debug/ and > > instead creates the directory under the correct node in > > /sys/kernel/debug/dri// > > So does this make it become /sys/kernel/debug/dri//debug? > > I wonder why it can't all be created under /sys/kernel/debug/dri/ > and then avoid the extra "debug" directory that isn't adding any value? > From the looks of it, it will still create the 'debug' dir, but at least under the correct .. for the record, I'm all for getting rid of the extra 'debug' directory, it saves me some extra typing ;-) BR, -R ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
