From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ramalingam C Subject: Re: [PATCH] drm/i915: Add debugfs entry for DRRS Date: Fri, 23 Jan 2015 23:17:04 +0530 Message-ID: <54C28918.1060400@intel.com> References: <54BF9B0C.8000803@intel.com> <1421945121-11050-1-git-send-email-ramalingam.c@intel.com> <20150123160330.GD10113@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1497437613==" Return-path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTP id 902396E92A for ; Fri, 23 Jan 2015 09:51:31 -0800 (PST) In-Reply-To: <20150123160330.GD10113@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter Cc: intel-gfx@lists.freedesktop.org, paulo.r.zanoni@intel.com, rodrigo.vivi@intel.com List-Id: intel-gfx@lists.freedesktop.org This is a multi-part message in MIME format. --===============1497437613== Content-Type: multipart/alternative; boundary="------------070101070006050708010107" This is a multi-part message in MIME format. --------------070101070006050708010107 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit On Friday 23 January 2015 09:33 PM, Daniel Vetter wrote: > On Thu, Jan 22, 2015 at 10:15:21PM +0530, Ramalingam C wrote: >> From: Vandana Kannan >> >> Adding a debugfs entry to determine if DRRS is supported or not >> >> V2: [By Ram]: Following details about the active crtc will be filled >> in seq-file of the debugfs >> 1. Encoder output type >> 2. DRRS Support on this CRTC >> 3. DRRS current state >> 4. Current Vrefresh >> Format is as follows: >> >> CRTC 1: Output: eDP, DRRS Supported: Yes (Seamless), DRRS_State: DRRS_HIGH_RR, Vrefresh: 60 >> CRTC 2: Output: HDMI, DRRS Supported : No, VBT DRRS_type: Seamless >> CRTC 1: Output: eDP, DRRS Supported: Yes (Seamless), DRRS_State: DRRS_LOW_RR, Vrefresh: 40 >> CRTC 2: Output: HDMI, DRRS Supported : No, VBT DRRS_type: Seamless >> >> Signed-off-by: Vandana Kannan >> Signed-off-by: Ramalingam C >> --- >> drivers/gpu/drm/i915/i915_debugfs.c | 93 +++++++++++++++++++++++++++++++++++ >> 1 file changed, 93 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c >> index 2ad4c48..47f1f65 100644 >> --- a/drivers/gpu/drm/i915/i915_debugfs.c >> +++ b/drivers/gpu/drm/i915/i915_debugfs.c >> @@ -2819,6 +2819,98 @@ static int i915_ddb_info(struct seq_file *m, void *unused) >> return 0; >> } >> >> +static int i915_drrs_status(struct seq_file *m, void *unused) >> +{ >> + struct drm_info_node *node = m->private; >> + struct drm_device *dev = node->minor->dev; >> + struct drm_i915_private *dev_priv = dev->dev_private; >> + struct i915_drrs *drrs = &dev_priv->drrs; >> + struct intel_crtc *intel_crtc; >> + struct intel_encoder *intel_encoder; >> + int active_crtc_cnt = 0, vrefresh = 0; >> + >> + for_each_intel_crtc(dev, intel_crtc) { >> + if (intel_crtc->active) { >> + active_crtc_cnt++; >> + seq_puts(m, "CRTC"); >> + seq_put_decimal_ull(m, ' ', active_crtc_cnt); >> + seq_puts(m, ": "); >> + for_each_encoder_on_crtc(dev, &intel_crtc->base, >> + intel_encoder) { >> + /* Encoder connected on this CRTC */ >> + switch (intel_encoder->type) { >> + case INTEL_OUTPUT_EDP: >> + seq_puts(m, "Output: eDP, "); >> + break; >> + case INTEL_OUTPUT_DSI: >> + seq_puts(m, "Output: DSI, "); >> + break; >> + case INTEL_OUTPUT_HDMI: >> + seq_puts(m, "Output: HDMI, "); >> + break; >> + case INTEL_OUTPUT_DISPLAYPORT: >> + seq_puts(m, "Output: DP, "); >> + break; >> + default: >> + seq_puts(m, "Output: Others (id"); >> + seq_put_decimal_ull(m, '=', >> + intel_encoder->type); >> + seq_puts(m, "), "); >> + } >> + } >> + >> + if (intel_crtc->config->has_drrs) { >> + struct intel_panel *panel; >> + >> + panel = &drrs->dp->attached_connector->panel; >> + /* DRRS Supported */ >> + seq_puts(m, >> + "DRRS Supported: Yes (Seamless), "); >> + if (drrs->refresh_rate_type == DRRS_HIGH_RR) { >> + seq_puts(m, >> + "DRRS_State: DRRS_HIGH_RR, "); >> + vrefresh = panel->fixed_mode->vrefresh; >> + } else if (drrs->refresh_rate_type == >> + DRRS_LOW_RR) { >> + seq_puts(m, >> + "DRRS_State: DRRS_LOW_RR, "); >> + vrefresh = >> + panel->downclock_mode->vrefresh; >> + } else { >> + seq_puts(m, "DRRS_State: Unknown"); >> + seq_put_decimal_ull(m, '(', >> + drrs->refresh_rate_type); >> + seq_puts(m, "), "); >> + } >> + seq_puts(m, "Vrefresh:"); >> + seq_put_decimal_ull(m, ' ', vrefresh); >> + >> + } else { >> + /* DRRS not supported. Print the VBT parameter*/ >> + seq_puts(m, "DRRS Supported : No, "); >> + if (dev_priv->vbt.drrs_type == >> + STATIC_DRRS_SUPPORT) { >> + seq_puts(m, >> + "VBT DRRS_type: Static"); >> + } else if (dev_priv->vbt.drrs_type == >> + SEAMLESS_DRRS_SUPPORT) { >> + seq_puts(m, >> + "VBT DRRS_type: Seamless"); >> + } else if (dev_priv->vbt.drrs_type == >> + DRRS_NOT_SUPPORTED) { >> + seq_puts(m, "VBT DRRS_type: None"); >> + } >> + } > This is an example of were strictly following checkpatch warnings and > pedantically breaking lines results in rather hard to read code: > The function is really long, and has about 6 indent levels. Imo it would > greatly benefit from extracting some helper functions to do the > inner-level printing. There's piles of examples in i915_debugfs where we > loop over a bunch of objects and punt all the real output to a small > helper function. > -Daniel Thanks Daniel. Submitting a patch in few mins. --Ram >> + seq_puts(m, "\n"); >> + } >> + } >> + >> + if (!active_crtc_cnt) >> + seq_puts(m, "No active crtc found\n"); >> + >> + return 0; >> +} >> + >> struct pipe_crc_info { >> const char *name; >> struct drm_device *dev; >> @@ -4433,6 +4525,7 @@ static const struct drm_info_list i915_debugfs_list[] = { >> {"i915_dp_mst_info", i915_dp_mst_info, 0}, >> {"i915_wa_registers", i915_wa_registers, 0}, >> {"i915_ddb_info", i915_ddb_info, 0}, >> + {"i915_drrs_status", i915_drrs_status, 0}, >> }; >> #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list) >> >> -- >> 1.7.9.5 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx --------------070101070006050708010107 Content-Type: text/html; charset=windows-1252 Content-Transfer-Encoding: 7bit
On Friday 23 January 2015 09:33 PM, Daniel Vetter wrote:
On Thu, Jan 22, 2015 at 10:15:21PM +0530, Ramalingam C wrote:

From: Vandana Kannan <vandana.kannan@intel.com>

Adding a debugfs entry to determine if DRRS is supported or not

V2: [By Ram]: Following details about the active crtc will be filled
	in seq-file of the debugfs
	1. Encoder output type
	2. DRRS Support on this CRTC
	3. DRRS current state
	4. Current Vrefresh
Format is as follows:

CRTC 1:  Output: eDP, DRRS Supported: Yes (Seamless), DRRS_State: DRRS_HIGH_RR, Vrefresh: 60
CRTC 2:  Output: HDMI, DRRS Supported : No, VBT DRRS_type: Seamless
CRTC 1:  Output: eDP, DRRS Supported: Yes (Seamless), DRRS_State: DRRS_LOW_RR, Vrefresh: 40
CRTC 2:  Output: HDMI, DRRS Supported : No, VBT DRRS_type: Seamless

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |   93 +++++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 2ad4c48..47f1f65 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2819,6 +2819,98 @@ static int i915_ddb_info(struct seq_file *m, void *unused)
 	return 0;
 }
 
+static int i915_drrs_status(struct seq_file *m, void *unused)
+{
+	struct drm_info_node *node = m->private;
+	struct drm_device *dev = node->minor->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct i915_drrs *drrs = &dev_priv->drrs;
+	struct intel_crtc *intel_crtc;
+	struct intel_encoder *intel_encoder;
+	int active_crtc_cnt = 0, vrefresh = 0;
+
+	for_each_intel_crtc(dev, intel_crtc) {
+		if (intel_crtc->active) {
+			active_crtc_cnt++;
+			seq_puts(m, "CRTC");
+			seq_put_decimal_ull(m, ' ', active_crtc_cnt);
+			seq_puts(m, ":  ");
+			for_each_encoder_on_crtc(dev, &intel_crtc->base,
+								intel_encoder) {
+				/* Encoder connected on this CRTC */
+				switch (intel_encoder->type) {
+				case INTEL_OUTPUT_EDP:
+					seq_puts(m, "Output: eDP, ");
+					break;
+				case INTEL_OUTPUT_DSI:
+					seq_puts(m, "Output: DSI, ");
+					break;
+				case INTEL_OUTPUT_HDMI:
+					seq_puts(m, "Output: HDMI, ");
+					break;
+				case INTEL_OUTPUT_DISPLAYPORT:
+					seq_puts(m, "Output: DP, ");
+					break;
+				default:
+					seq_puts(m, "Output: Others (id");
+					seq_put_decimal_ull(m, '=',
+							intel_encoder->type);
+					seq_puts(m, "), ");
+				}
+			}
+
+			if (intel_crtc->config->has_drrs) {
+				struct intel_panel *panel;
+
+				panel = &drrs->dp->attached_connector->panel;
+				/* DRRS Supported */
+				seq_puts(m,
+					"DRRS Supported: Yes (Seamless), ");
+				if (drrs->refresh_rate_type == DRRS_HIGH_RR) {
+					seq_puts(m,
+						"DRRS_State: DRRS_HIGH_RR, ");
+					vrefresh = panel->fixed_mode->vrefresh;
+				} else if (drrs->refresh_rate_type ==
+								DRRS_LOW_RR) {
+					seq_puts(m,
+						"DRRS_State: DRRS_LOW_RR, ");
+					vrefresh =
+						panel->downclock_mode->vrefresh;
+				} else {
+					seq_puts(m, "DRRS_State: Unknown");
+					seq_put_decimal_ull(m, '(',
+						drrs->refresh_rate_type);
+					seq_puts(m, "), ");
+				}
+				seq_puts(m, "Vrefresh:");
+				seq_put_decimal_ull(m, ' ', vrefresh);
+
+			} else {
+				/* DRRS not supported. Print the VBT parameter*/
+				seq_puts(m, "DRRS Supported : No, ");
+				if (dev_priv->vbt.drrs_type ==
+							STATIC_DRRS_SUPPORT) {
+					seq_puts(m,
+						"VBT DRRS_type: Static");
+				} else if (dev_priv->vbt.drrs_type ==
+							SEAMLESS_DRRS_SUPPORT) {
+					seq_puts(m,
+						"VBT DRRS_type: Seamless");
+				} else if (dev_priv->vbt.drrs_type ==
+							DRRS_NOT_SUPPORTED) {
+					seq_puts(m, "VBT DRRS_type: None");
+				}
+			}

This is an example of were strictly following checkpatch warnings and
pedantically breaking lines results in rather hard to read code:
The function is really long, and has about 6 indent levels. Imo it would
greatly benefit from extracting some helper functions to do the
inner-level printing. There's piles of examples in i915_debugfs where we
loop over a bunch of objects and punt all the real output to a small
helper function.
-Daniel
Thanks Daniel. Submitting a patch in few mins. --Ram 
+			seq_puts(m, "\n");
+		}
+	}
+
+	if (!active_crtc_cnt)
+		seq_puts(m, "No active crtc found\n");
+
+	return 0;
+}
+
 struct pipe_crc_info {
 	const char *name;
 	struct drm_device *dev;
@@ -4433,6 +4525,7 @@ static const struct drm_info_list i915_debugfs_list[] = {
 	{"i915_dp_mst_info", i915_dp_mst_info, 0},
 	{"i915_wa_registers", i915_wa_registers, 0},
 	{"i915_ddb_info", i915_ddb_info, 0},
+	{"i915_drrs_status", i915_drrs_status, 0},
 };
 #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
 
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


    

    

  


--------------070101070006050708010107--

--===============1497437613==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: base64
Content-Disposition: inline

X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4
IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0
cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK

--===============1497437613==--