| Message ID | 20201106101443.686053-2-gwan-gyeong.mun@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On Fri, 06 Nov 2020, Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> wrote: > In order to support the PSR state of each transcoder, it adds > i915_psr_status to sub-directory of each transcoder. > > v2: Change using of Symbolic permissions 'S_IRUGO' to using of octal > permissions '0444' > > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> > Cc: José Roberto de Souza <jose.souza@intel.com> > --- > .../drm/i915/display/intel_display_debugfs.c | 23 +++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > index 8402e6ac9f76..37805615a221 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > @@ -2093,6 +2093,23 @@ static int i915_hdcp_sink_capability_show(struct seq_file *m, void *data) > } > DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability); > > +static int i915_psr_status_show(struct seq_file *m, void *data) > +{ > + struct drm_connector *connector = m->private; > + struct intel_dp *intel_dp = > + intel_attached_dp(to_intel_connector(connector)); > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > + > + if (connector->status != connector_status_connected) > + return -ENODEV; > + > + if (!HAS_PSR(dev_priv)) > + return -ENODEV; > + > + return intel_psr_status(m, intel_dp); > +} > +DEFINE_SHOW_ATTRIBUTE(i915_psr_status); > + > #define LPSP_CAPABLE(COND) (COND ? seq_puts(m, "LPSP: capable\n") : \ > seq_puts(m, "LPSP: incapable\n")) > > @@ -2268,6 +2285,12 @@ int intel_connector_debugfs_add(struct drm_connector *connector) > connector, &i915_psr_sink_status_fops); > } > > + if (INTEL_GEN(dev_priv) >= 12 && I'd add this for all generations to unify the debugfs, and eventually phase out the non connector specific debugfs file. And I'd add HAS_PSR() check here to not create the file if it's not possible instead of having the check in i915_psr_status_show(). BR, Jani. > + connector->connector_type == DRM_MODE_CONNECTOR_eDP) { > + debugfs_create_file("i915_psr_status", 0444, root, > + connector, &i915_psr_status_fops); > + } > + > if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort || > connector->connector_type == DRM_MODE_CONNECTOR_HDMIA || > connector->connector_type == DRM_MODE_CONNECTOR_HDMIB) {
On Fri, 06 Nov 2020, Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> wrote: > In order to support the PSR state of each transcoder, it adds > i915_psr_status to sub-directory of each transcoder. > > v2: Change using of Symbolic permissions 'S_IRUGO' to using of octal > permissions '0444' > > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> > Cc: José Roberto de Souza <jose.souza@intel.com> > --- > .../drm/i915/display/intel_display_debugfs.c | 23 +++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > index 8402e6ac9f76..37805615a221 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > @@ -2093,6 +2093,23 @@ static int i915_hdcp_sink_capability_show(struct seq_file *m, void *data) > } > DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability); > > +static int i915_psr_status_show(struct seq_file *m, void *data) > +{ > + struct drm_connector *connector = m->private; > + struct intel_dp *intel_dp = > + intel_attached_dp(to_intel_connector(connector)); > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > + > + if (connector->status != connector_status_connected) How's this possible for eDP, btw? BR, Jani. > + return -ENODEV; > + > + if (!HAS_PSR(dev_priv)) > + return -ENODEV; > + > + return intel_psr_status(m, intel_dp); > +} > +DEFINE_SHOW_ATTRIBUTE(i915_psr_status); > + > #define LPSP_CAPABLE(COND) (COND ? seq_puts(m, "LPSP: capable\n") : \ > seq_puts(m, "LPSP: incapable\n")) > > @@ -2268,6 +2285,12 @@ int intel_connector_debugfs_add(struct drm_connector *connector) > connector, &i915_psr_sink_status_fops); > } > > + if (INTEL_GEN(dev_priv) >= 12 && > + connector->connector_type == DRM_MODE_CONNECTOR_eDP) { > + debugfs_create_file("i915_psr_status", 0444, root, > + connector, &i915_psr_status_fops); > + } > + > if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort || > connector->connector_type == DRM_MODE_CONNECTOR_HDMIA || > connector->connector_type == DRM_MODE_CONNECTOR_HDMIB) {
On 2020-11-18 at 16:42:29 +0530, Jani Nikula wrote: > On Fri, 06 Nov 2020, Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> wrote: > > In order to support the PSR state of each transcoder, it adds > > i915_psr_status to sub-directory of each transcoder. > > > > v2: Change using of Symbolic permissions 'S_IRUGO' to using of octal > > permissions '0444' > > > > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> > > Cc: José Roberto de Souza <jose.souza@intel.com> > > --- > > .../drm/i915/display/intel_display_debugfs.c | 23 +++++++++++++++++++ > > 1 file changed, 23 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > index 8402e6ac9f76..37805615a221 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > @@ -2093,6 +2093,23 @@ static int i915_hdcp_sink_capability_show(struct seq_file *m, void *data) > > } > > DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability); > > > > +static int i915_psr_status_show(struct seq_file *m, void *data) > > +{ > > + struct drm_connector *connector = m->private; > > + struct intel_dp *intel_dp = > > + intel_attached_dp(to_intel_connector(connector)); > > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > + > > + if (connector->status != connector_status_connected) > > How's this possible for eDP, btw? > > BR, > Jani. > > > + return -ENODEV; > > + > > + if (!HAS_PSR(dev_priv)) > > + return -ENODEV; > > + > > + return intel_psr_status(m, intel_dp); > > +} > > +DEFINE_SHOW_ATTRIBUTE(i915_psr_status); > > + > > #define LPSP_CAPABLE(COND) (COND ? seq_puts(m, "LPSP: capable\n") : \ > > seq_puts(m, "LPSP: incapable\n")) > > > > @@ -2268,6 +2285,12 @@ int intel_connector_debugfs_add(struct drm_connector *connector) > > connector, &i915_psr_sink_status_fops); > > } > > > > + if (INTEL_GEN(dev_priv) >= 12 && > > + connector->connector_type == DRM_MODE_CONNECTOR_eDP) { Hi GG IMHO this should connector->connector_type == DRM_MODE_CONNECTOR_eDP || connector->connector_type == DRM_MODE_SUBCONNECTOR_DisplayPort to support DP Panel Reply, i read somewere DP panel reply is PSR with Link Full ON ? I believe this would be the reason to keep file name as "i915_psr_status" instead of i915_edp_psr_status? Thanks, Anshuman. > > + debugfs_create_file("i915_psr_status", 0444, root, > > + connector, &i915_psr_status_fops); > > + } > > + > > if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort || > > connector->connector_type == DRM_MODE_CONNECTOR_HDMIA || > > connector->connector_type == DRM_MODE_CONNECTOR_HDMIB) { > > -- > Jani Nikula, Intel Open Source Graphics Center > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, 2020-11-18 at 13:11 +0200, Jani Nikula wrote: > On Fri, 06 Nov 2020, Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> > wrote: > > In order to support the PSR state of each transcoder, it adds > > i915_psr_status to sub-directory of each transcoder. > > > > v2: Change using of Symbolic permissions 'S_IRUGO' to using of > > octal > > permissions '0444' > > > > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> > > Cc: José Roberto de Souza <jose.souza@intel.com> > > --- > > .../drm/i915/display/intel_display_debugfs.c | 23 > > +++++++++++++++++++ > > 1 file changed, 23 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > index 8402e6ac9f76..37805615a221 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > @@ -2093,6 +2093,23 @@ static int > > i915_hdcp_sink_capability_show(struct seq_file *m, void *data) > > } > > DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability); > > > > +static int i915_psr_status_show(struct seq_file *m, void *data) > > +{ > > + struct drm_connector *connector = m->private; > > + struct intel_dp *intel_dp = > > + intel_attached_dp(to_intel_connector(connector)); > > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > + > > + if (connector->status != connector_status_connected) > > + return -ENODEV; > > + > > + if (!HAS_PSR(dev_priv)) > > + return -ENODEV; > > + > > + return intel_psr_status(m, intel_dp); > > +} > > +DEFINE_SHOW_ATTRIBUTE(i915_psr_status); > > + > > #define LPSP_CAPABLE(COND) (COND ? seq_puts(m, "LPSP: capable\n") > > : \ > > seq_puts(m, "LPSP: incapable\n")) > > > > @@ -2268,6 +2285,12 @@ int intel_connector_debugfs_add(struct > > drm_connector *connector) > > connector, > > &i915_psr_sink_status_fops); > > } > > > > + if (INTEL_GEN(dev_priv) >= 12 && > > I'd add this for all generations to unify the debugfs, and eventually > phase out the non connector specific debugfs file. > > And I'd add HAS_PSR() check here to not create the file if it's not > possible instead of having the check in i915_psr_status_show(). > Hi Jani, Thank you for checking the patch. I'll update the code as per your recommendations. > BR, > Jani. > > > + connector->connector_type == DRM_MODE_CONNECTOR_eDP) { > > + debugfs_create_file("i915_psr_status", 0444, root, > > + connector, &i915_psr_status_fops); > > + } > > + > > if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort > > || > > connector->connector_type == DRM_MODE_CONNECTOR_HDMIA || > > connector->connector_type == DRM_MODE_CONNECTOR_HDMIB) {
On Fri, 2020-12-04 at 21:36 +0530, Anshuman Gupta wrote: > On 2020-11-18 at 16:42:29 +0530, Jani Nikula wrote: > > On Fri, 06 Nov 2020, Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> > > wrote: > > > In order to support the PSR state of each transcoder, it adds > > > i915_psr_status to sub-directory of each transcoder. > > > > > > v2: Change using of Symbolic permissions 'S_IRUGO' to using of > > > octal > > > permissions '0444' > > > > > > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> > > > Cc: José Roberto de Souza <jose.souza@intel.com> > > > --- > > > .../drm/i915/display/intel_display_debugfs.c | 23 > > > +++++++++++++++++++ > > > 1 file changed, 23 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > > index 8402e6ac9f76..37805615a221 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > > @@ -2093,6 +2093,23 @@ static int > > > i915_hdcp_sink_capability_show(struct seq_file *m, void *data) > > > } > > > DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability); > > > > > > +static int i915_psr_status_show(struct seq_file *m, void *data) > > > +{ > > > + struct drm_connector *connector = m->private; > > > + struct intel_dp *intel_dp = > > > + intel_attached_dp(to_intel_connector(connector)); > > > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > > + > > > + if (connector->status != connector_status_connected) > > > > How's this possible for eDP, btw? > > When I wrote the code first it was considered PanelReplay of DP spec. for now it is not needed for edp. > > BR, > > Jani. > > > > > + return -ENODEV; > > > + > > > + if (!HAS_PSR(dev_priv)) > > > + return -ENODEV; > > > + > > > + return intel_psr_status(m, intel_dp); > > > +} > > > +DEFINE_SHOW_ATTRIBUTE(i915_psr_status); > > > + > > > #define LPSP_CAPABLE(COND) (COND ? seq_puts(m, "LPSP: > > > capable\n") : \ > > > seq_puts(m, "LPSP: incapable\n")) > > > > > > @@ -2268,6 +2285,12 @@ int intel_connector_debugfs_add(struct > > > drm_connector *connector) > > > connector, > > > &i915_psr_sink_status_fops); > > > } > > > > > > + if (INTEL_GEN(dev_priv) >= 12 && > > > + connector->connector_type == DRM_MODE_CONNECTOR_eDP) { > Hi GG > IMHO this should connector->connector_type == DRM_MODE_CONNECTOR_eDP > || connector->connector_type == DRM_MODE_SUBCONNECTOR_DisplayPort > to support DP Panel Reply, i read somewere DP panel reply is PSR with > Link Full ON ? > I believe this would be the reason to keep file name as > "i915_psr_status" instead of i915_edp_psr_status? Hi Anshuman, Yes, the file name (i915_psr_status) was considered for DP PanelReplay too. But for now, i915 is not supported PanelReplay yet, it would be better to limit it to edp. When the PanelReplay is ready I'll update here too. > Thanks, > Anshuman. > > > + debugfs_create_file("i915_psr_status", 0444, root, > > > + connector, &i915_psr_status_fops); > > > + } > > > + > > > if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort > > > || > > > connector->connector_type == DRM_MODE_CONNECTOR_HDMIA || > > > connector->connector_type == DRM_MODE_CONNECTOR_HDMIB) { > > > > -- > > Jani Nikula, Intel Open Source Graphics Center > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c index 8402e6ac9f76..37805615a221 100644 --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c @@ -2093,6 +2093,23 @@ static int i915_hdcp_sink_capability_show(struct seq_file *m, void *data) } DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability); +static int i915_psr_status_show(struct seq_file *m, void *data) +{ + struct drm_connector *connector = m->private; + struct intel_dp *intel_dp = + intel_attached_dp(to_intel_connector(connector)); + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); + + if (connector->status != connector_status_connected) + return -ENODEV; + + if (!HAS_PSR(dev_priv)) + return -ENODEV; + + return intel_psr_status(m, intel_dp); +} +DEFINE_SHOW_ATTRIBUTE(i915_psr_status); + #define LPSP_CAPABLE(COND) (COND ? seq_puts(m, "LPSP: capable\n") : \ seq_puts(m, "LPSP: incapable\n")) @@ -2268,6 +2285,12 @@ int intel_connector_debugfs_add(struct drm_connector *connector) connector, &i915_psr_sink_status_fops); } + if (INTEL_GEN(dev_priv) >= 12 && + connector->connector_type == DRM_MODE_CONNECTOR_eDP) { + debugfs_create_file("i915_psr_status", 0444, root, + connector, &i915_psr_status_fops); + } + if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort || connector->connector_type == DRM_MODE_CONNECTOR_HDMIA || connector->connector_type == DRM_MODE_CONNECTOR_HDMIB) {
In order to support the PSR state of each transcoder, it adds i915_psr_status to sub-directory of each transcoder. v2: Change using of Symbolic permissions 'S_IRUGO' to using of octal permissions '0444' Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> Cc: José Roberto de Souza <jose.souza@intel.com> --- .../drm/i915/display/intel_display_debugfs.c | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+)