* [PATCH v2] drm/msm/dp: Only create debugfs for PRIMARY minor
@ 2021-12-17 0:20 ` Bjorn Andersson
0 siblings, 0 replies; 8+ messages in thread
From: Bjorn Andersson @ 2021-12-17 0:20 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov
Cc: Sean Paul, David Airlie, Daniel Vetter, linux-arm-msm, dri-devel,
freedreno, linux-kernel
dpu_kms_debugfs_init() is invoked for each minor being registered. Most
of the files created are unrelated to the minor, so there's no reason to
present them per minor.
The exception to this is the DisplayPort code, which ends up invoking
dp_debug_get() for each minor, each time associate the allocated object
with dp->debug.
As such dp_debug will create debugfs files in both the PRIMARY and the
RENDER minor's debugfs directory, but only the last reference will be
remembered.
The only use of this reference today is in the cleanup path in
dp_display_deinit_sub_modules() and the dp_debug_private object does
outlive the debugfs entries in either case, so there doesn't seem to be
any adverse effects of this, but per the code the current behavior is
unexpected, so change it to only create debugfs files for the PRIMARY
minor.
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
Changes since v1:
- Moved the check up from msm_dp_debugfs_init() to dpu_kms_debugfs_init()
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 2ee70072a1b4..a54f7d373f14 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -193,6 +193,10 @@ static int dpu_kms_debugfs_init(struct msm_kms *kms, struct drm_minor *minor)
if (!p)
return -EINVAL;
+ /* Only create one set of debugfs per DP instance */
+ if (minor->type != DRM_MINOR_PRIMARY)
+ return 0;
+
dev = dpu_kms->dev;
priv = dev->dev_private;
--
2.33.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2] drm/msm/dp: Only create debugfs for PRIMARY minor
@ 2021-12-17 0:20 ` Bjorn Andersson
0 siblings, 0 replies; 8+ messages in thread
From: Bjorn Andersson @ 2021-12-17 0:20 UTC (permalink / raw)
To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov
Cc: Sean Paul, David Airlie, linux-arm-msm, linux-kernel, dri-devel,
freedreno
dpu_kms_debugfs_init() is invoked for each minor being registered. Most
of the files created are unrelated to the minor, so there's no reason to
present them per minor.
The exception to this is the DisplayPort code, which ends up invoking
dp_debug_get() for each minor, each time associate the allocated object
with dp->debug.
As such dp_debug will create debugfs files in both the PRIMARY and the
RENDER minor's debugfs directory, but only the last reference will be
remembered.
The only use of this reference today is in the cleanup path in
dp_display_deinit_sub_modules() and the dp_debug_private object does
outlive the debugfs entries in either case, so there doesn't seem to be
any adverse effects of this, but per the code the current behavior is
unexpected, so change it to only create debugfs files for the PRIMARY
minor.
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
Changes since v1:
- Moved the check up from msm_dp_debugfs_init() to dpu_kms_debugfs_init()
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 2ee70072a1b4..a54f7d373f14 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -193,6 +193,10 @@ static int dpu_kms_debugfs_init(struct msm_kms *kms, struct drm_minor *minor)
if (!p)
return -EINVAL;
+ /* Only create one set of debugfs per DP instance */
+ if (minor->type != DRM_MINOR_PRIMARY)
+ return 0;
+
dev = dpu_kms->dev;
priv = dev->dev_private;
--
2.33.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] drm/msm/dp: Only create debugfs for PRIMARY minor
2021-12-17 0:20 ` Bjorn Andersson
@ 2021-12-20 19:51 ` Abhinav Kumar
-1 siblings, 0 replies; 8+ messages in thread
From: Abhinav Kumar @ 2021-12-20 19:51 UTC (permalink / raw)
To: Bjorn Andersson, Rob Clark, Dmitry Baryshkov
Cc: Sean Paul, David Airlie, Daniel Vetter, linux-arm-msm, dri-devel,
freedreno, linux-kernel
On 12/16/2021 4:20 PM, Bjorn Andersson wrote:
> dpu_kms_debugfs_init() is invoked for each minor being registered. Most
> of the files created are unrelated to the minor, so there's no reason to
> present them per minor.
> The exception to this is the DisplayPort code, which ends up invoking
> dp_debug_get() for each minor, each time associate the allocated object
> with dp->debug.
>
> As such dp_debug will create debugfs files in both the PRIMARY and the
> RENDER minor's debugfs directory, but only the last reference will be
> remembered.
>
> The only use of this reference today is in the cleanup path in
> dp_display_deinit_sub_modules() and the dp_debug_private object does
> outlive the debugfs entries in either case, so there doesn't seem to be
> any adverse effects of this, but per the code the current behavior is
> unexpected, so change it to only create debugfs files for the PRIMARY
> minor.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>
> Changes since v1:
> - Moved the check up from msm_dp_debugfs_init() to dpu_kms_debugfs_init()
>
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 2ee70072a1b4..a54f7d373f14 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -193,6 +193,10 @@ static int dpu_kms_debugfs_init(struct msm_kms *kms, struct drm_minor *minor)
> if (!p)
> return -EINVAL;
>
> + /* Only create one set of debugfs per DP instance */
I would change this to DPU now instead of DP, but apart from that,
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> + if (minor->type != DRM_MINOR_PRIMARY)
> + return 0;
> +
> dev = dpu_kms->dev;
> priv = dev->dev_private;
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] drm/msm/dp: Only create debugfs for PRIMARY minor
@ 2021-12-20 19:51 ` Abhinav Kumar
0 siblings, 0 replies; 8+ messages in thread
From: Abhinav Kumar @ 2021-12-20 19:51 UTC (permalink / raw)
To: Bjorn Andersson, Rob Clark, Dmitry Baryshkov
Cc: Sean Paul, David Airlie, linux-arm-msm, linux-kernel, dri-devel,
freedreno
On 12/16/2021 4:20 PM, Bjorn Andersson wrote:
> dpu_kms_debugfs_init() is invoked for each minor being registered. Most
> of the files created are unrelated to the minor, so there's no reason to
> present them per minor.
> The exception to this is the DisplayPort code, which ends up invoking
> dp_debug_get() for each minor, each time associate the allocated object
> with dp->debug.
>
> As such dp_debug will create debugfs files in both the PRIMARY and the
> RENDER minor's debugfs directory, but only the last reference will be
> remembered.
>
> The only use of this reference today is in the cleanup path in
> dp_display_deinit_sub_modules() and the dp_debug_private object does
> outlive the debugfs entries in either case, so there doesn't seem to be
> any adverse effects of this, but per the code the current behavior is
> unexpected, so change it to only create debugfs files for the PRIMARY
> minor.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>
> Changes since v1:
> - Moved the check up from msm_dp_debugfs_init() to dpu_kms_debugfs_init()
>
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 2ee70072a1b4..a54f7d373f14 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -193,6 +193,10 @@ static int dpu_kms_debugfs_init(struct msm_kms *kms, struct drm_minor *minor)
> if (!p)
> return -EINVAL;
>
> + /* Only create one set of debugfs per DP instance */
I would change this to DPU now instead of DP, but apart from that,
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> + if (minor->type != DRM_MINOR_PRIMARY)
> + return 0;
> +
> dev = dpu_kms->dev;
> priv = dev->dev_private;
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] drm/msm/dp: Only create debugfs for PRIMARY minor
2021-12-17 0:20 ` Bjorn Andersson
@ 2021-12-20 23:53 ` Dmitry Baryshkov
-1 siblings, 0 replies; 8+ messages in thread
From: Dmitry Baryshkov @ 2021-12-20 23:53 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Rob Clark, Abhinav Kumar, Sean Paul, David Airlie, Daniel Vetter,
linux-arm-msm, dri-devel, freedreno, linux-kernel
On Fri, 17 Dec 2021 at 03:19, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> dpu_kms_debugfs_init() is invoked for each minor being registered. Most
> of the files created are unrelated to the minor, so there's no reason to
> present them per minor.
> The exception to this is the DisplayPort code, which ends up invoking
> dp_debug_get() for each minor, each time associate the allocated object
> with dp->debug.
>
> As such dp_debug will create debugfs files in both the PRIMARY and the
> RENDER minor's debugfs directory, but only the last reference will be
> remembered.
>
> The only use of this reference today is in the cleanup path in
> dp_display_deinit_sub_modules() and the dp_debug_private object does
> outlive the debugfs entries in either case, so there doesn't seem to be
> any adverse effects of this, but per the code the current behavior is
> unexpected, so change it to only create debugfs files for the PRIMARY
> minor.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>
> Changes since v1:
> - Moved the check up from msm_dp_debugfs_init() to dpu_kms_debugfs_init()
>
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 2ee70072a1b4..a54f7d373f14 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -193,6 +193,10 @@ static int dpu_kms_debugfs_init(struct msm_kms *kms, struct drm_minor *minor)
> if (!p)
> return -EINVAL;
>
> + /* Only create one set of debugfs per DP instance */
The comment is misleading. Could you please fix it?
> + if (minor->type != DRM_MINOR_PRIMARY)
> + return 0;
> +
> dev = dpu_kms->dev;
> priv = dev->dev_private;
>
> --
> 2.33.1
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] drm/msm/dp: Only create debugfs for PRIMARY minor
@ 2021-12-20 23:53 ` Dmitry Baryshkov
0 siblings, 0 replies; 8+ messages in thread
From: Dmitry Baryshkov @ 2021-12-20 23:53 UTC (permalink / raw)
To: Bjorn Andersson
Cc: freedreno, David Airlie, linux-arm-msm, Abhinav Kumar, dri-devel,
linux-kernel, Sean Paul
On Fri, 17 Dec 2021 at 03:19, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> dpu_kms_debugfs_init() is invoked for each minor being registered. Most
> of the files created are unrelated to the minor, so there's no reason to
> present them per minor.
> The exception to this is the DisplayPort code, which ends up invoking
> dp_debug_get() for each minor, each time associate the allocated object
> with dp->debug.
>
> As such dp_debug will create debugfs files in both the PRIMARY and the
> RENDER minor's debugfs directory, but only the last reference will be
> remembered.
>
> The only use of this reference today is in the cleanup path in
> dp_display_deinit_sub_modules() and the dp_debug_private object does
> outlive the debugfs entries in either case, so there doesn't seem to be
> any adverse effects of this, but per the code the current behavior is
> unexpected, so change it to only create debugfs files for the PRIMARY
> minor.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>
> Changes since v1:
> - Moved the check up from msm_dp_debugfs_init() to dpu_kms_debugfs_init()
>
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 2ee70072a1b4..a54f7d373f14 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -193,6 +193,10 @@ static int dpu_kms_debugfs_init(struct msm_kms *kms, struct drm_minor *minor)
> if (!p)
> return -EINVAL;
>
> + /* Only create one set of debugfs per DP instance */
The comment is misleading. Could you please fix it?
> + if (minor->type != DRM_MINOR_PRIMARY)
> + return 0;
> +
> dev = dpu_kms->dev;
> priv = dev->dev_private;
>
> --
> 2.33.1
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] drm/msm/dp: Only create debugfs for PRIMARY minor
2021-12-20 23:53 ` Dmitry Baryshkov
@ 2021-12-21 0:02 ` Bjorn Andersson
-1 siblings, 0 replies; 8+ messages in thread
From: Bjorn Andersson @ 2021-12-21 0:02 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Rob Clark, Abhinav Kumar, Sean Paul, David Airlie, Daniel Vetter,
linux-arm-msm, dri-devel, freedreno, linux-kernel
On Mon 20 Dec 15:53 PST 2021, Dmitry Baryshkov wrote:
> On Fri, 17 Dec 2021 at 03:19, Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> >
> > dpu_kms_debugfs_init() is invoked for each minor being registered. Most
> > of the files created are unrelated to the minor, so there's no reason to
> > present them per minor.
> > The exception to this is the DisplayPort code, which ends up invoking
> > dp_debug_get() for each minor, each time associate the allocated object
> > with dp->debug.
> >
> > As such dp_debug will create debugfs files in both the PRIMARY and the
> > RENDER minor's debugfs directory, but only the last reference will be
> > remembered.
> >
> > The only use of this reference today is in the cleanup path in
> > dp_display_deinit_sub_modules() and the dp_debug_private object does
> > outlive the debugfs entries in either case, so there doesn't seem to be
> > any adverse effects of this, but per the code the current behavior is
> > unexpected, so change it to only create debugfs files for the PRIMARY
> > minor.
> >
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> >
> > Changes since v1:
> > - Moved the check up from msm_dp_debugfs_init() to dpu_kms_debugfs_init()
> >
> > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > index 2ee70072a1b4..a54f7d373f14 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > @@ -193,6 +193,10 @@ static int dpu_kms_debugfs_init(struct msm_kms *kms, struct drm_minor *minor)
> > if (!p)
> > return -EINVAL;
> >
> > + /* Only create one set of debugfs per DP instance */
>
> The comment is misleading. Could you please fix it?
>
I agree, and as Abhinav pointed out I didn't update $subject fully
either.
Will resubmit.
Regards,
Bjorn
> > + if (minor->type != DRM_MINOR_PRIMARY)
> > + return 0;
> > +
> > dev = dpu_kms->dev;
> > priv = dev->dev_private;
> >
> > --
> > 2.33.1
> >
>
>
> --
> With best wishes
> Dmitry
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] drm/msm/dp: Only create debugfs for PRIMARY minor
@ 2021-12-21 0:02 ` Bjorn Andersson
0 siblings, 0 replies; 8+ messages in thread
From: Bjorn Andersson @ 2021-12-21 0:02 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: freedreno, David Airlie, linux-arm-msm, Abhinav Kumar, dri-devel,
linux-kernel, Sean Paul
On Mon 20 Dec 15:53 PST 2021, Dmitry Baryshkov wrote:
> On Fri, 17 Dec 2021 at 03:19, Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> >
> > dpu_kms_debugfs_init() is invoked for each minor being registered. Most
> > of the files created are unrelated to the minor, so there's no reason to
> > present them per minor.
> > The exception to this is the DisplayPort code, which ends up invoking
> > dp_debug_get() for each minor, each time associate the allocated object
> > with dp->debug.
> >
> > As such dp_debug will create debugfs files in both the PRIMARY and the
> > RENDER minor's debugfs directory, but only the last reference will be
> > remembered.
> >
> > The only use of this reference today is in the cleanup path in
> > dp_display_deinit_sub_modules() and the dp_debug_private object does
> > outlive the debugfs entries in either case, so there doesn't seem to be
> > any adverse effects of this, but per the code the current behavior is
> > unexpected, so change it to only create debugfs files for the PRIMARY
> > minor.
> >
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> >
> > Changes since v1:
> > - Moved the check up from msm_dp_debugfs_init() to dpu_kms_debugfs_init()
> >
> > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > index 2ee70072a1b4..a54f7d373f14 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > @@ -193,6 +193,10 @@ static int dpu_kms_debugfs_init(struct msm_kms *kms, struct drm_minor *minor)
> > if (!p)
> > return -EINVAL;
> >
> > + /* Only create one set of debugfs per DP instance */
>
> The comment is misleading. Could you please fix it?
>
I agree, and as Abhinav pointed out I didn't update $subject fully
either.
Will resubmit.
Regards,
Bjorn
> > + if (minor->type != DRM_MINOR_PRIMARY)
> > + return 0;
> > +
> > dev = dpu_kms->dev;
> > priv = dev->dev_private;
> >
> > --
> > 2.33.1
> >
>
>
> --
> With best wishes
> Dmitry
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-12-21 0:01 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-17 0:20 [PATCH v2] drm/msm/dp: Only create debugfs for PRIMARY minor Bjorn Andersson
2021-12-17 0:20 ` Bjorn Andersson
2021-12-20 19:51 ` Abhinav Kumar
2021-12-20 19:51 ` Abhinav Kumar
2021-12-20 23:53 ` Dmitry Baryshkov
2021-12-20 23:53 ` Dmitry Baryshkov
2021-12-21 0:02 ` Bjorn Andersson
2021-12-21 0:02 ` Bjorn Andersson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.