| Message ID | 20240424183820.3591593-7-animesh.manna@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Link off between frames for edp | expand |
On Thu, 2024-04-25 at 00:08 +0530, Animesh Manna wrote: > For validation purpose add debugfs for LOBF. > > Signed-off-by: Animesh Manna <animesh.manna@intel.com> > --- > drivers/gpu/drm/i915/display/intel_alpm.c | 48 > +++++++++++++++++++ > drivers/gpu/drm/i915/display/intel_alpm.h | 2 + > .../drm/i915/display/intel_display_debugfs.c | 2 + > 3 files changed, 52 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c > b/drivers/gpu/drm/i915/display/intel_alpm.c > index b08799586b58..2d3027c2fb0a 100644 > --- a/drivers/gpu/drm/i915/display/intel_alpm.c > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c > @@ -343,3 +343,51 @@ void intel_alpm_configure(struct intel_dp > *intel_dp, > { > lnl_alpm_configure(intel_dp, crtc_state); > } > + > +static int i915_edp_lobf_info_show(struct seq_file *m, void *data) > +{ > + struct intel_connector *connector = m->private; > + struct drm_i915_private *dev_priv = to_i915(connector- > >base.dev); > + struct drm_crtc *crtc; > + struct intel_crtc_state *crtc_state; > + enum transcoder cpu_transcoder; > + bool lobf_enabled; > + int ret; > + > + ret = drm_modeset_lock_single_interruptible(&dev_priv- > >drm.mode_config.connection_mutex); > + if (ret) > + return ret; > + > + crtc = connector->base.state->crtc; > + if (connector->base.status != connector_status_connected || > !crtc) { > + ret = -ENODEV; > + goto out; > + } > + > + crtc_state = to_intel_crtc_state(crtc->state); > + seq_printf(m, "LOBF Criteria met: %s\n", > str_yes_no(crtc_state->has_lobf)); I'm still not convinced on this. has_lobf ~= ALPM_CTL_LOBF_ENABLE in ALPM_CTL. To my opinion it is enough to dump seq_printf(m, "LOBF status: %s\n", str_enabled_disabled(lobf_enabled)) below. Maybe AUX_WAKE and AUX_LESS_WAKE could be dumped instead? BR, Jouni Högander > + > + cpu_transcoder = crtc_state->cpu_transcoder; > + lobf_enabled = intel_de_read(dev_priv, > ALPM_CTL(cpu_transcoder)) & ALPM_CTL_LOBF_ENABLE; > + seq_printf(m, "LOBF status: %s\n", > str_enabled_disabled(lobf_enabled)); > + > +out: > + drm_modeset_unlock(&dev_priv- > >drm.mode_config.connection_mutex); > + > + return ret; > +} > + > +DEFINE_SHOW_ATTRIBUTE(i915_edp_lobf_info); > + > +void intel_alpm_lobf_debugfs_add(struct intel_connector *connector) > +{ > + struct drm_i915_private *i915 = to_i915(connector->base.dev); > + struct dentry *root = connector->base.debugfs_entry; > + > + if (DISPLAY_VER(i915) < 20 || > + connector->base.connector_type != DRM_MODE_CONNECTOR_eDP) > + return; > + > + debugfs_create_file("i915_edp_lobf_info", 0444, root, > + connector, &i915_edp_lobf_info_fops); > +} > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.h > b/drivers/gpu/drm/i915/display/intel_alpm.h > index a9ca190da3e4..01fd08eb96f5 100644 > --- a/drivers/gpu/drm/i915/display/intel_alpm.h > +++ b/drivers/gpu/drm/i915/display/intel_alpm.h > @@ -11,6 +11,7 @@ > struct intel_dp; > struct intel_crtc_state; > struct drm_connector_state; > +struct intel_connector; > > bool intel_alpm_get_aux_less_status(struct intel_dp *intel_dp); > bool intel_alpm_compute_params(struct intel_dp *intel_dp, > @@ -20,4 +21,5 @@ void intel_alpm_compute_lobf_config(struct intel_dp > *intel_dp, > struct drm_connector_state > *conn_state); > void intel_alpm_configure(struct intel_dp *intel_dp, > const struct intel_crtc_state *crtc_state); > +void intel_alpm_lobf_debugfs_add(struct intel_connector *connector); > #endif > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > index 35f9f86ef70f..86d9900c40af 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > @@ -13,6 +13,7 @@ > #include "i915_debugfs.h" > #include "i915_irq.h" > #include "i915_reg.h" > +#include "intel_alpm.h" > #include "intel_crtc.h" > #include "intel_de.h" > #include "intel_crtc_state_dump.h" > @@ -1515,6 +1516,7 @@ void intel_connector_debugfs_add(struct > intel_connector *connector) > intel_drrs_connector_debugfs_add(connector); > intel_pps_connector_debugfs_add(connector); > intel_psr_connector_debugfs_add(connector); > + intel_alpm_lobf_debugfs_add(connector); > > if (connector_type == DRM_MODE_CONNECTOR_DisplayPort || > connector_type == DRM_MODE_CONNECTOR_HDMIA ||
> -----Original Message----- > From: Hogander, Jouni <jouni.hogander@intel.com> > Sent: Friday, May 3, 2024 1:02 PM > To: Manna, Animesh <animesh.manna@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org; Murthy, Arun R > <arun.r.murthy@intel.com>; Nikula, Jani <jani.nikula@intel.com> > Subject: Re: [PATCH v3 6/6] drm/i915/alpm: Add debugfs for LOBF > > On Thu, 2024-04-25 at 00:08 +0530, Animesh Manna wrote: > > For validation purpose add debugfs for LOBF. > > > > Signed-off-by: Animesh Manna <animesh.manna@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_alpm.c | 48 > > +++++++++++++++++++ > > drivers/gpu/drm/i915/display/intel_alpm.h | 2 + > > .../drm/i915/display/intel_display_debugfs.c | 2 + > > 3 files changed, 52 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c > > b/drivers/gpu/drm/i915/display/intel_alpm.c > > index b08799586b58..2d3027c2fb0a 100644 > > --- a/drivers/gpu/drm/i915/display/intel_alpm.c > > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c > > @@ -343,3 +343,51 @@ void intel_alpm_configure(struct intel_dp > > *intel_dp, > > { > > lnl_alpm_configure(intel_dp, crtc_state); > > } > > + > > +static int i915_edp_lobf_info_show(struct seq_file *m, void *data) > > +{ > > + struct intel_connector *connector = m->private; > > + struct drm_i915_private *dev_priv = to_i915(connector- > > >base.dev); > > + struct drm_crtc *crtc; > > + struct intel_crtc_state *crtc_state; > > + enum transcoder cpu_transcoder; > > + bool lobf_enabled; > > + int ret; > > + > > + ret = drm_modeset_lock_single_interruptible(&dev_priv- > > >drm.mode_config.connection_mutex); > > + if (ret) > > + return ret; > > + > > + crtc = connector->base.state->crtc; > > + if (connector->base.status != connector_status_connected || > > !crtc) { > > + ret = -ENODEV; > > + goto out; > > + } > > + > > + crtc_state = to_intel_crtc_state(crtc->state); > > + seq_printf(m, "LOBF Criteria met: %s\n", > > str_yes_no(crtc_state->has_lobf)); > > I'm still not convinced on this. has_lobf ~= ALPM_CTL_LOBF_ENABLE in > ALPM_CTL. To my opinion it is enough to dump seq_printf(m, "LOBF > status: %s\n", str_enabled_disabled(lobf_enabled)) below. Maybe > AUX_WAKE and AUX_LESS_WAKE could be dumped instead? Can add aux-wake or aux-less info as well. But as LOBF feature is dependent on adaptive sync fixed refresh rate mode (which can be dynamic as per user input) and ALPM. I want to expose both the conditions are satisfying or not along with status. Regards, Animesh > > BR, > > Jouni Högander > > > + > > + cpu_transcoder = crtc_state->cpu_transcoder; > > + lobf_enabled = intel_de_read(dev_priv, > > ALPM_CTL(cpu_transcoder)) & ALPM_CTL_LOBF_ENABLE; > > + seq_printf(m, "LOBF status: %s\n", > > str_enabled_disabled(lobf_enabled)); > > + > > +out: > > + drm_modeset_unlock(&dev_priv- > > >drm.mode_config.connection_mutex); > > + > > + return ret; > > +} > > + > > +DEFINE_SHOW_ATTRIBUTE(i915_edp_lobf_info); > > + > > +void intel_alpm_lobf_debugfs_add(struct intel_connector *connector) > > +{ > > + struct drm_i915_private *i915 = to_i915(connector->base.dev); > > + struct dentry *root = connector->base.debugfs_entry; > > + > > + if (DISPLAY_VER(i915) < 20 || > > + connector->base.connector_type != > DRM_MODE_CONNECTOR_eDP) > > + return; > > + > > + debugfs_create_file("i915_edp_lobf_info", 0444, root, > > + connector, &i915_edp_lobf_info_fops); > > +} > > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.h > > b/drivers/gpu/drm/i915/display/intel_alpm.h > > index a9ca190da3e4..01fd08eb96f5 100644 > > --- a/drivers/gpu/drm/i915/display/intel_alpm.h > > +++ b/drivers/gpu/drm/i915/display/intel_alpm.h > > @@ -11,6 +11,7 @@ > > struct intel_dp; > > struct intel_crtc_state; > > struct drm_connector_state; > > +struct intel_connector; > > > > bool intel_alpm_get_aux_less_status(struct intel_dp *intel_dp); > > bool intel_alpm_compute_params(struct intel_dp *intel_dp, > > @@ -20,4 +21,5 @@ void intel_alpm_compute_lobf_config(struct intel_dp > > *intel_dp, > > struct drm_connector_state > > *conn_state); > > void intel_alpm_configure(struct intel_dp *intel_dp, > > const struct intel_crtc_state *crtc_state); > > +void intel_alpm_lobf_debugfs_add(struct intel_connector *connector); > > #endif > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > index 35f9f86ef70f..86d9900c40af 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > @@ -13,6 +13,7 @@ > > #include "i915_debugfs.h" > > #include "i915_irq.h" > > #include "i915_reg.h" > > +#include "intel_alpm.h" > > #include "intel_crtc.h" > > #include "intel_de.h" > > #include "intel_crtc_state_dump.h" > > @@ -1515,6 +1516,7 @@ void intel_connector_debugfs_add(struct > > intel_connector *connector) > > intel_drrs_connector_debugfs_add(connector); > > intel_pps_connector_debugfs_add(connector); > > intel_psr_connector_debugfs_add(connector); > > + intel_alpm_lobf_debugfs_add(connector); > > > > if (connector_type == DRM_MODE_CONNECTOR_DisplayPort || > > connector_type == DRM_MODE_CONNECTOR_HDMIA ||
On Fri, 2024-05-03 at 08:30 +0000, Manna, Animesh wrote: > > > > -----Original Message----- > > From: Hogander, Jouni <jouni.hogander@intel.com> > > Sent: Friday, May 3, 2024 1:02 PM > > To: Manna, Animesh <animesh.manna@intel.com>; intel- > > gfx@lists.freedesktop.org > > Cc: dri-devel@lists.freedesktop.org; Murthy, Arun R > > <arun.r.murthy@intel.com>; Nikula, Jani <jani.nikula@intel.com> > > Subject: Re: [PATCH v3 6/6] drm/i915/alpm: Add debugfs for LOBF > > > > On Thu, 2024-04-25 at 00:08 +0530, Animesh Manna wrote: > > > For validation purpose add debugfs for LOBF. > > > > > > Signed-off-by: Animesh Manna <animesh.manna@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_alpm.c | 48 > > > +++++++++++++++++++ > > > drivers/gpu/drm/i915/display/intel_alpm.h | 2 + > > > .../drm/i915/display/intel_display_debugfs.c | 2 + > > > 3 files changed, 52 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c > > > b/drivers/gpu/drm/i915/display/intel_alpm.c > > > index b08799586b58..2d3027c2fb0a 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_alpm.c > > > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c > > > @@ -343,3 +343,51 @@ void intel_alpm_configure(struct intel_dp > > > *intel_dp, > > > { > > > lnl_alpm_configure(intel_dp, crtc_state); > > > } > > > + > > > +static int i915_edp_lobf_info_show(struct seq_file *m, void > > > *data) > > > +{ > > > + struct intel_connector *connector = m->private; > > > + struct drm_i915_private *dev_priv = to_i915(connector- > > > > base.dev); > > > + struct drm_crtc *crtc; > > > + struct intel_crtc_state *crtc_state; > > > + enum transcoder cpu_transcoder; > > > + bool lobf_enabled; > > > + int ret; > > > + > > > + ret = drm_modeset_lock_single_interruptible(&dev_priv- > > > > drm.mode_config.connection_mutex); > > > + if (ret) > > > + return ret; > > > + > > > + crtc = connector->base.state->crtc; > > > + if (connector->base.status != connector_status_connected > > > || > > > !crtc) { > > > + ret = -ENODEV; > > > + goto out; > > > + } > > > + > > > + crtc_state = to_intel_crtc_state(crtc->state); > > > + seq_printf(m, "LOBF Criteria met: %s\n", > > > str_yes_no(crtc_state->has_lobf)); > > > > I'm still not convinced on this. has_lobf ~= ALPM_CTL_LOBF_ENABLE > > in > > ALPM_CTL. To my opinion it is enough to dump seq_printf(m, "LOBF > > status: %s\n", str_enabled_disabled(lobf_enabled)) below. Maybe > > AUX_WAKE and AUX_LESS_WAKE could be dumped instead? > > Can add aux-wake or aux-less info as well. > But as LOBF feature is dependent on adaptive sync fixed refresh rate > mode (which can be dynamic as per user input) and ALPM. I want to > expose both the conditions are satisfying or not along with status. If all those conditions are satisfied (i.e. has_lobf is true) then ALPM_CTL & ALPM_CTL_LOBF_ENABLE is true? So I'm wondering what is the benefit from dumping has_lobf? BR, Jouni Högander > > Regards, > Animesh > > > > > BR, > > > > Jouni Högander > > > > > + > > > + cpu_transcoder = crtc_state->cpu_transcoder; > > > + lobf_enabled = intel_de_read(dev_priv, > > > ALPM_CTL(cpu_transcoder)) & ALPM_CTL_LOBF_ENABLE; > > > + seq_printf(m, "LOBF status: %s\n", > > > str_enabled_disabled(lobf_enabled)); > > > + > > > +out: > > > + drm_modeset_unlock(&dev_priv- > > > > drm.mode_config.connection_mutex); > > > + > > > + return ret; > > > +} > > > + > > > +DEFINE_SHOW_ATTRIBUTE(i915_edp_lobf_info); > > > + > > > +void intel_alpm_lobf_debugfs_add(struct intel_connector > > > *connector) > > > +{ > > > + struct drm_i915_private *i915 = to_i915(connector- > > > >base.dev); > > > + struct dentry *root = connector->base.debugfs_entry; > > > + > > > + if (DISPLAY_VER(i915) < 20 || > > > + connector->base.connector_type != > > DRM_MODE_CONNECTOR_eDP) > > > + return; > > > + > > > + debugfs_create_file("i915_edp_lobf_info", 0444, root, > > > + connector, &i915_edp_lobf_info_fops); > > > +} > > > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.h > > > b/drivers/gpu/drm/i915/display/intel_alpm.h > > > index a9ca190da3e4..01fd08eb96f5 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_alpm.h > > > +++ b/drivers/gpu/drm/i915/display/intel_alpm.h > > > @@ -11,6 +11,7 @@ > > > struct intel_dp; > > > struct intel_crtc_state; > > > struct drm_connector_state; > > > +struct intel_connector; > > > > > > bool intel_alpm_get_aux_less_status(struct intel_dp *intel_dp); > > > bool intel_alpm_compute_params(struct intel_dp *intel_dp, > > > @@ -20,4 +21,5 @@ void intel_alpm_compute_lobf_config(struct > > > intel_dp > > > *intel_dp, > > > struct drm_connector_state > > > *conn_state); > > > void intel_alpm_configure(struct intel_dp *intel_dp, > > > const struct intel_crtc_state > > > *crtc_state); > > > +void intel_alpm_lobf_debugfs_add(struct intel_connector > > > *connector); > > > #endif > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > > index 35f9f86ef70f..86d9900c40af 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > > > @@ -13,6 +13,7 @@ > > > #include "i915_debugfs.h" > > > #include "i915_irq.h" > > > #include "i915_reg.h" > > > +#include "intel_alpm.h" > > > #include "intel_crtc.h" > > > #include "intel_de.h" > > > #include "intel_crtc_state_dump.h" > > > @@ -1515,6 +1516,7 @@ void intel_connector_debugfs_add(struct > > > intel_connector *connector) > > > intel_drrs_connector_debugfs_add(connector); > > > intel_pps_connector_debugfs_add(connector); > > > intel_psr_connector_debugfs_add(connector); > > > + intel_alpm_lobf_debugfs_add(connector); > > > > > > if (connector_type == DRM_MODE_CONNECTOR_DisplayPort || > > > connector_type == DRM_MODE_CONNECTOR_HDMIA || >
diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c b/drivers/gpu/drm/i915/display/intel_alpm.c index b08799586b58..2d3027c2fb0a 100644 --- a/drivers/gpu/drm/i915/display/intel_alpm.c +++ b/drivers/gpu/drm/i915/display/intel_alpm.c @@ -343,3 +343,51 @@ void intel_alpm_configure(struct intel_dp *intel_dp, { lnl_alpm_configure(intel_dp, crtc_state); } + +static int i915_edp_lobf_info_show(struct seq_file *m, void *data) +{ + struct intel_connector *connector = m->private; + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); + struct drm_crtc *crtc; + struct intel_crtc_state *crtc_state; + enum transcoder cpu_transcoder; + bool lobf_enabled; + int ret; + + ret = drm_modeset_lock_single_interruptible(&dev_priv->drm.mode_config.connection_mutex); + if (ret) + return ret; + + crtc = connector->base.state->crtc; + if (connector->base.status != connector_status_connected || !crtc) { + ret = -ENODEV; + goto out; + } + + crtc_state = to_intel_crtc_state(crtc->state); + seq_printf(m, "LOBF Criteria met: %s\n", str_yes_no(crtc_state->has_lobf)); + + cpu_transcoder = crtc_state->cpu_transcoder; + lobf_enabled = intel_de_read(dev_priv, ALPM_CTL(cpu_transcoder)) & ALPM_CTL_LOBF_ENABLE; + seq_printf(m, "LOBF status: %s\n", str_enabled_disabled(lobf_enabled)); + +out: + drm_modeset_unlock(&dev_priv->drm.mode_config.connection_mutex); + + return ret; +} + +DEFINE_SHOW_ATTRIBUTE(i915_edp_lobf_info); + +void intel_alpm_lobf_debugfs_add(struct intel_connector *connector) +{ + struct drm_i915_private *i915 = to_i915(connector->base.dev); + struct dentry *root = connector->base.debugfs_entry; + + if (DISPLAY_VER(i915) < 20 || + connector->base.connector_type != DRM_MODE_CONNECTOR_eDP) + return; + + debugfs_create_file("i915_edp_lobf_info", 0444, root, + connector, &i915_edp_lobf_info_fops); +} diff --git a/drivers/gpu/drm/i915/display/intel_alpm.h b/drivers/gpu/drm/i915/display/intel_alpm.h index a9ca190da3e4..01fd08eb96f5 100644 --- a/drivers/gpu/drm/i915/display/intel_alpm.h +++ b/drivers/gpu/drm/i915/display/intel_alpm.h @@ -11,6 +11,7 @@ struct intel_dp; struct intel_crtc_state; struct drm_connector_state; +struct intel_connector; bool intel_alpm_get_aux_less_status(struct intel_dp *intel_dp); bool intel_alpm_compute_params(struct intel_dp *intel_dp, @@ -20,4 +21,5 @@ void intel_alpm_compute_lobf_config(struct intel_dp *intel_dp, struct drm_connector_state *conn_state); void intel_alpm_configure(struct intel_dp *intel_dp, const struct intel_crtc_state *crtc_state); +void intel_alpm_lobf_debugfs_add(struct intel_connector *connector); #endif diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c index 35f9f86ef70f..86d9900c40af 100644 --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c @@ -13,6 +13,7 @@ #include "i915_debugfs.h" #include "i915_irq.h" #include "i915_reg.h" +#include "intel_alpm.h" #include "intel_crtc.h" #include "intel_de.h" #include "intel_crtc_state_dump.h" @@ -1515,6 +1516,7 @@ void intel_connector_debugfs_add(struct intel_connector *connector) intel_drrs_connector_debugfs_add(connector); intel_pps_connector_debugfs_add(connector); intel_psr_connector_debugfs_add(connector); + intel_alpm_lobf_debugfs_add(connector); if (connector_type == DRM_MODE_CONNECTOR_DisplayPort || connector_type == DRM_MODE_CONNECTOR_HDMIA ||
For validation purpose add debugfs for LOBF. Signed-off-by: Animesh Manna <animesh.manna@intel.com> --- drivers/gpu/drm/i915/display/intel_alpm.c | 48 +++++++++++++++++++ drivers/gpu/drm/i915/display/intel_alpm.h | 2 + .../drm/i915/display/intel_display_debugfs.c | 2 + 3 files changed, 52 insertions(+)