Re: [PATCH] drm/i915/dp: Fix the PSR debugfs entries wrt. MST connectors
On Wed, 2024-01-03 at 16:00 +0200, Imre Deak wrote:
> On Wed, Jan 03, 2024 at 01:37:08PM +0200, Hogander, Jouni wrote:
> > > > > [...]
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > index 494d08817d71e..b5b9340e505e3 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > @@ -3310,11 +3310,8 @@ void
> > > > > intel_psr_connector_debugfs_add(struct
> > > > > intel_connector *connector)
> > > > > struct drm_i915_private *i915 = to_i915(connector-
> > > > > >base.dev);
> > > > > struct dentry *root = connector->base.debugfs_entry;
> > > > >
> > > > > - if (connector->base.connector_type !=
> > > > > DRM_MODE_CONNECTOR_eDP)
> > > > > {
> > > > > - if (!(HAS_DP20(i915) &&
> > > > > - connector->base.connector_type ==
> > > > > DRM_MODE_CONNECTOR_DisplayPort))
> > > > > - return;
> > > > > - }
> > > > > + if (connector->base.connector_type !=
> > > > > DRM_MODE_CONNECTOR_eDP)
> > > > > + return;
> > > >
> > > > Would it be possible to disable it only for MST connector? I
> > > > think
> > > > this is disabling it also for DP SST, no?
> > >
> > > Yes, it keeps it enabled only for eDP. It could be enabled for
> > > SST as
> > > well yes, but I thought as a fix the above is better, adding
> > > support
> > > for other connector types as a follow up.
> >
> > if (connector->mst_port || !(HAS_DP20(i915) &&
> > connectorbase.connector_type == DRM_MODE_CONNECTOR_DisplayPort))
> > return;
> >
> > Is it possible to use this instead?
>
> Looking through it I don't see a problem on SST connectors either, so
> I'd rather leave the entries enabled for them on all platforms, that
> is
>
> if ((connector_type != DRM_MODE_CONNECTOR_eDP &&
> connector_type != DRM_MODE_CONNECTOR_DisplayPort) ||
> connector->mst_port)
> return;
Sounds good. That is anyways same what is done for PSR as well.
BR,
Jouni Högander
>
> > BR,
> >
> > Jouni Högander
> >
> > >
> > > > BR,
> > > >
> > > > Jouni Högander
> > > > >
> > > > > debugfs_create_file("i915_psr_sink_status", 0444,
> > > > > root,
> > > > > connector,
> > > > > &i915_psr_sink_status_fops);
> > > >
> >
Re: [PATCH] drm/i915/dp: Fix the PSR debugfs entries wrt. MST connectors
On Wed, Jan 03, 2024 at 01:37:08PM +0200, Hogander, Jouni wrote:
> > > > [...]
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > index 494d08817d71e..b5b9340e505e3 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > @@ -3310,11 +3310,8 @@ void
> > > > intel_psr_connector_debugfs_add(struct
> > > > intel_connector *connector)
> > > > struct drm_i915_private *i915 = to_i915(connector->base.dev);
> > > > struct dentry *root = connector->base.debugfs_entry;
> > > >
> > > > - if (connector->base.connector_type != DRM_MODE_CONNECTOR_eDP)
> > > > {
> > > > - if (!(HAS_DP20(i915) &&
> > > > - connector->base.connector_type ==
> > > > DRM_MODE_CONNECTOR_DisplayPort))
> > > > - return;
> > > > - }
> > > > + if (connector->base.connector_type != DRM_MODE_CONNECTOR_eDP)
> > > > + return;
> > >
> > > Would it be possible to disable it only for MST connector? I think
> > > this is disabling it also for DP SST, no?
> >
> > Yes, it keeps it enabled only for eDP. It could be enabled for SST as
> > well yes, but I thought as a fix the above is better, adding support
> > for other connector types as a follow up.
>
> if (connector->mst_port || !(HAS_DP20(i915) &&
> connectorbase.connector_type == DRM_MODE_CONNECTOR_DisplayPort))
> return;
>
> Is it possible to use this instead?
Looking through it I don't see a problem on SST connectors either, so
I'd rather leave the entries enabled for them on all platforms, that is
if ((connector_type != DRM_MODE_CONNECTOR_eDP &&
connector_type != DRM_MODE_CONNECTOR_DisplayPort) ||
connector->mst_port)
return;
> BR,
>
> Jouni Högander
>
> >
> > > BR,
> > >
> > > Jouni Högander
> > > >
> > > > debugfs_create_file("i915_psr_sink_status", 0444, root,
> > > > connector,
> > > > &i915_psr_sink_status_fops);
> > >
>
Re: [PATCH] drm/i915/dp: Fix the PSR debugfs entries wrt. MST connectors
On Wed, 2024-01-03 at 13:20 +0200, Imre Deak wrote:
> On Wed, Jan 03, 2024 at 01:08:05PM +0200, Hogander, Jouni wrote:
> > On Wed, 2024-01-03 at 13:00 +0200, Imre Deak wrote:
> > > MST connectors don't have a static attached encoder, as their
> > > encoder
> > > can change depending on the pipe they use; so the encoder for an
> > > MST
> > > connector can't be retrieved using intel_dp_attached_encoder()
> > > (which
> > > may return NULL for MST). Most of the PSR debugfs entries depend
> > > on a
> > > static connector -> encoder mapping which is only true for eDP
> > > and
> > > SST
> > > DP connectors and not for MST. These debugfs entries were enabled
> > > for
> > > MST connectors as well recently to provide PR information for
> > > them,
> > > but
> > > handling MST connectors needs more changes. Fix this by re-
> > > disabling
> > > for
> > > now the PSR entries on MST connectors.
> > >
> > > Fixes: ef75c25e8fed ("drm/i915/panelreplay: Debugfs support for
> > > panel
> > > replay")
> > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9850
> > > Cc: Animesh Manna
> > > Cc: Jouni Högander
> > > Signed-off-by: Imre Deak
> > > ---
> > > drivers/gpu/drm/i915/display/intel_psr.c | 7 ++-
> > > 1 file changed, 2 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > index 494d08817d71e..b5b9340e505e3 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -3310,11 +3310,8 @@ void
> > > intel_psr_connector_debugfs_add(struct
> > > intel_connector *connector)
> > > struct drm_i915_private *i915 = to_i915(connector-
> > > >base.dev);
> > > struct dentry *root = connector->base.debugfs_entry;
> > >
> > > - if (connector->base.connector_type !=
> > > DRM_MODE_CONNECTOR_eDP)
> > > {
> > > - if (!(HAS_DP20(i915) &&
> > > - connector->base.connector_type ==
> > > DRM_MODE_CONNECTOR_DisplayPort))
> > > - return;
> > > - }
> > > + if (connector->base.connector_type !=
> > > DRM_MODE_CONNECTOR_eDP)
> > > + return;
> >
> > Would it be possible to disable it only for MST connector? I think
> > this
> > is disabling it also for DP SST, no?
>
> Yes, it keeps it enabled only for eDP. It could be enabled for SST as
> well yes, but I thought as a fix the above is better, adding support
> for
> other connector types as a follow up.
if (connector->mst_port || !(HAS_DP20(i915) &&
connectorbase.connector_type == DRM_MODE_CONNECTOR_DisplayPort))
return;
Is it possible to use this instead?
BR,
Jouni Högander
>
> > BR,
> >
> > Jouni Högander
> > >
> > > debugfs_create_file("i915_psr_sink_status", 0444, root,
> > > connector,
> > > &i915_psr_sink_status_fops);
> >
Re: [PATCH] drm/i915/dp: Fix the PSR debugfs entries wrt. MST connectors
On Wed, Jan 03, 2024 at 01:08:05PM +0200, Hogander, Jouni wrote:
> On Wed, 2024-01-03 at 13:00 +0200, Imre Deak wrote:
> > MST connectors don't have a static attached encoder, as their encoder
> > can change depending on the pipe they use; so the encoder for an MST
> > connector can't be retrieved using intel_dp_attached_encoder() (which
> > may return NULL for MST). Most of the PSR debugfs entries depend on a
> > static connector -> encoder mapping which is only true for eDP and
> > SST
> > DP connectors and not for MST. These debugfs entries were enabled for
> > MST connectors as well recently to provide PR information for them,
> > but
> > handling MST connectors needs more changes. Fix this by re-disabling
> > for
> > now the PSR entries on MST connectors.
> >
> > Fixes: ef75c25e8fed ("drm/i915/panelreplay: Debugfs support for panel
> > replay")
> > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9850
> > Cc: Animesh Manna
> > Cc: Jouni Högander
> > Signed-off-by: Imre Deak
> > ---
> > drivers/gpu/drm/i915/display/intel_psr.c | 7 ++-
> > 1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 494d08817d71e..b5b9340e505e3 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -3310,11 +3310,8 @@ void intel_psr_connector_debugfs_add(struct
> > intel_connector *connector)
> > struct drm_i915_private *i915 = to_i915(connector->base.dev);
> > struct dentry *root = connector->base.debugfs_entry;
> >
> > - if (connector->base.connector_type != DRM_MODE_CONNECTOR_eDP)
> > {
> > - if (!(HAS_DP20(i915) &&
> > - connector->base.connector_type ==
> > DRM_MODE_CONNECTOR_DisplayPort))
> > - return;
> > - }
> > + if (connector->base.connector_type != DRM_MODE_CONNECTOR_eDP)
> > + return;
>
> Would it be possible to disable it only for MST connector? I think this
> is disabling it also for DP SST, no?
Yes, it keeps it enabled only for eDP. It could be enabled for SST as
well yes, but I thought as a fix the above is better, adding support for
other connector types as a follow up.
> BR,
>
> Jouni Högander
> >
> > debugfs_create_file("i915_psr_sink_status", 0444, root,
> > connector, &i915_psr_sink_status_fops);
>
Re: [PATCH] drm/i915/dp: Fix the PSR debugfs entries wrt. MST connectors
On Wed, 2024-01-03 at 13:00 +0200, Imre Deak wrote:
> MST connectors don't have a static attached encoder, as their encoder
> can change depending on the pipe they use; so the encoder for an MST
> connector can't be retrieved using intel_dp_attached_encoder() (which
> may return NULL for MST). Most of the PSR debugfs entries depend on a
> static connector -> encoder mapping which is only true for eDP and
> SST
> DP connectors and not for MST. These debugfs entries were enabled for
> MST connectors as well recently to provide PR information for them,
> but
> handling MST connectors needs more changes. Fix this by re-disabling
> for
> now the PSR entries on MST connectors.
>
> Fixes: ef75c25e8fed ("drm/i915/panelreplay: Debugfs support for panel
> replay")
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9850
> Cc: Animesh Manna
> Cc: Jouni Högander
> Signed-off-by: Imre Deak
> ---
> drivers/gpu/drm/i915/display/intel_psr.c | 7 ++-
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 494d08817d71e..b5b9340e505e3 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -3310,11 +3310,8 @@ void intel_psr_connector_debugfs_add(struct
> intel_connector *connector)
> struct drm_i915_private *i915 = to_i915(connector->base.dev);
> struct dentry *root = connector->base.debugfs_entry;
>
> - if (connector->base.connector_type != DRM_MODE_CONNECTOR_eDP)
> {
> - if (!(HAS_DP20(i915) &&
> - connector->base.connector_type ==
> DRM_MODE_CONNECTOR_DisplayPort))
> - return;
> - }
> + if (connector->base.connector_type != DRM_MODE_CONNECTOR_eDP)
> + return;
Would it be possible to disable it only for MST connector? I think this
is disabling it also for DP SST, no?
BR,
Jouni Högander
>
> debugfs_create_file("i915_psr_sink_status", 0444, root,
> connector, &i915_psr_sink_status_fops);
[PATCH] drm/i915/dp: Fix the PSR debugfs entries wrt. MST connectors
MST connectors don't have a static attached encoder, as their encoder
can change depending on the pipe they use; so the encoder for an MST
connector can't be retrieved using intel_dp_attached_encoder() (which
may return NULL for MST). Most of the PSR debugfs entries depend on a
static connector -> encoder mapping which is only true for eDP and SST
DP connectors and not for MST. These debugfs entries were enabled for
MST connectors as well recently to provide PR information for them, but
handling MST connectors needs more changes. Fix this by re-disabling for
now the PSR entries on MST connectors.
Fixes: ef75c25e8fed ("drm/i915/panelreplay: Debugfs support for panel replay")
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9850
Cc: Animesh Manna
Cc: Jouni Högander
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/display/intel_psr.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
b/drivers/gpu/drm/i915/display/intel_psr.c
index 494d08817d71e..b5b9340e505e3 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -3310,11 +3310,8 @@ void intel_psr_connector_debugfs_add(struct
intel_connector *connector)
struct drm_i915_private *i915 = to_i915(connector->base.dev);
struct dentry *root = connector->base.debugfs_entry;
- if (connector->base.connector_type != DRM_MODE_CONNECTOR_eDP) {
- if (!(HAS_DP20(i915) &&
- connector->base.connector_type ==
DRM_MODE_CONNECTOR_DisplayPort))
- return;
- }
+ if (connector->base.connector_type != DRM_MODE_CONNECTOR_eDP)
+ return;
debugfs_create_file("i915_psr_sink_status", 0444, root,
connector, &i915_psr_sink_status_fops);
--
2.39.2
