[Intel-gfx] [PATCH v4 2/4] drm/i915: Add i915_lpsp_capability debugfs

[Anshuman Gupta]

On 2020-04-14 at 21:16:00 +0530, Manna, Animesh wrote:
> 
> On 09-04-2020 11:36, Anshuman Gupta wrote:
> >New i915_pm_lpsp igt solution approach relies on connector specific
> >debugfs attribute i915_lpsp_capability, it exposes whether an output is
> >capable of driving lpsp.
> >
> >v2:
> >- CI fixup.
> >v3:
> >- register i915_lpsp_info only for supported connector. [Jani]
> >- use intel_display_power_well_is_enabled() instead of looking
> >   inside power_well count. [Jani]
> >- fixes the lpsp capable conditional logic. [Jani]
> >- combined the lpsp capable and enable info. [Jani]
> >v4:
> >- Separate out connector based debugfs i915_lpsp_capability
> >   lpsp enable status would be exposes by different entry. [Animesh]
> >
> >Signed-off-by: Anshuman Gupta <anshuman.gupta at intel.com>
> >---
> >  .../drm/i915/display/intel_display_debugfs.c  | 63 +++++++++++++++++++
> >  1 file changed, 63 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> >index bdeea2e02642..402b89daff62 100644
> >--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> >+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> >@@ -611,6 +611,28 @@ static void intel_hdcp_info(struct seq_file *m,
> >  	seq_puts(m, "\n");
> >  }
> >+#define LPSP_CAPABLE(COND) (COND ? seq_puts(m, "LPSP: capable\n") : \
> >+				seq_puts(m, "LPSP: incapable\n"))
> >+
> >+static bool intel_have_edp_dsi_panel(struct drm_connector *connector)
> >+{
> >+	return connector->connector_type == DRM_MODE_CONNECTOR_DSI ||
> >+		connector->connector_type == DRM_MODE_CONNECTOR_eDP;
> >+}
> >+
> >+static bool intel_have_dp_edp_dsi_panel(struct drm_connector *connector)
> >+{
> >+	return intel_have_edp_dsi_panel(connector) ||
> >+		connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort;
> >+}
> >+
> >+static bool intel_have_hdmi_dp_edp_dsi_panel(struct drm_connector *connector)
> >+{
> >+	return intel_have_dp_edp_dsi_panel(connector) ||
> >+		connector->connector_type == DRM_MODE_CONNECTOR_HDMIA ||
> >+		connector->connector_type == DRM_MODE_CONNECTOR_HDMIB;
> >+}
> >+
> >  static void intel_dp_info(struct seq_file *m,
> >  			  struct intel_connector *intel_connector)
> >  {
> >@@ -1991,6 +2013,42 @@ static int i915_hdcp_sink_capability_show(struct seq_file *m, void *data)
> >  }
> >  DEFINE_SHOW_ATTRIBUTE(i915_hdcp_sink_capability);
> >+static int i915_lpsp_capability_show(struct seq_file *m, void *data)
> >+{
> >+	struct drm_connector *connector = m->private;
> >+	struct intel_encoder *encoder =
> >+			intel_attached_encoder(to_intel_connector(connector));
> >+	struct drm_i915_private *i915 = to_i915(connector->dev);
> >+
> >+	if (connector->status != connector_status_connected)
> >+		return -ENODEV;
> >+
> >+	switch (INTEL_GEN(i915)) {
> >+	case 12:
> >+		/*
> >+		 * Actually TGL can drive LPSP on port till DDI_C
> >+		 * but there is no physical connected DDI_C on TGL sku's,
> >+		 * even driver is not initilizing DDI_C port for gen12.
> >+		 */
> >+		LPSP_CAPABLE(encoder->port <= PORT_B);
> >+		break;
> >+	case 11:
> >+		LPSP_CAPABLE(intel_have_edp_dsi_panel(connector));
> >+		break;
> >+	case 10:
> >+	case 9:
> >+		LPSP_CAPABLE(encoder->port == PORT_A &&
> >+			     intel_have_dp_edp_dsi_panel(connector));
> >+		break;
> >+	default:
> >+		if (IS_HASWELL(i915) || IS_BROADWELL(i915))
> >+			LPSP_CAPABLE(connector->connector_type == DRM_MODE_CONNECTOR_eDP);
> >+	}
> >+
> >+	return 0;
> >+}
> >+DEFINE_SHOW_ATTRIBUTE(i915_lpsp_capability);
> >+
> >  static int i915_dsc_fec_support_show(struct seq_file *m, void *data)
> >  {
> >  	struct drm_connector *connector = m->private;
> >@@ -2134,5 +2192,10 @@ int intel_connector_debugfs_add(struct drm_connector *connector)
> >  		debugfs_create_file("i915_dsc_fec_support", S_IRUGO, root,
> >  				    connector, &i915_dsc_fec_support_fops);
> >+	/* Legacy panels doesn't lpsp on any platform */
> >+	if (intel_have_hdmi_dp_edp_dsi_panel(connector))
> >+		debugfs_create_file("i915_lpsp_capability", 0444, root,
> >+				    connector, &i915_lpsp_capability_fops);
> 
> Need a condition check of INTEL_GEN(dev_priv), for older platform the entry will get created and later in i915_lpsp_capability_show() will not print anything.
Thanks for review and catching this animesh, i overlooked it with thought that, older gen before HASWELL/BROADWELL won't support eDP/DSI/DP/HDMI connector.
May be a condition INTEL_GEN(dev_priv) > 9 || HASWELL(dev_priv)  || BROADWELL(dev_priv) will do the job.
becuase i am not really sure about the legacy gen number.
> With the above fix, Reviewed-by: Animesh Manna <animesh.manna at intel.com>
> Nitpick: imo adding function for different combination of connector type check maybe overdoing. In i915_lpsp_capability_show(), can have <gen>_lpsp_cap_check() for different generation.
I want to fix this but may be i am not able to understand it, <gen>_lpsp_cap_check will also need to check for connector with respect to gen, like ex on Gen11 only eDP and DSI can drive lpsp, so we would require to check connector  connector->connector_type == eDP || connector->connector_type == DSI.

Thanks,
Anshuman.
> 
> Regards,
> Animesh
> 
> >+
> >  	return 0;
> >  }