On 10/26/2017 10:54 AM, Daniele Ceraolo Spurio wrote:
On 25/10/17 06:31, Michal Wajdeczko wrote:
On Tue, 24 Oct 2017 19:21:20 +0200, Sujaritha Sundaresan
<sujaritha.sundaresan@xxxxxxxxx> wrote:
Unifying the various seq_puts messages in debugfs to the simplest
one for
feature support.
v2: Clarifying the commit message (Anusha)
v3: Re-factoring code as per review (Michal)
v4: Rebase
v5: Split from following patch
v6: Re-factoring code (Michal, Sagar)
Clarifying commit message (Sagar)
v7: Generalizing subject to drm/i915 (Sagar)
v8: Omitting DRRS seq_puts unification (Michal)
Suggested by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@xxxxxxxxx>
Cc: Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx>
Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Oscar Mateo <oscar.mateo@xxxxxxxxx>
Cc: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_debugfs.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index c65e381..8edd029 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1641,7 +1641,7 @@ static int i915_fbc_status(struct seq_file *m,
void *unused)
struct drm_i915_private *dev_priv = node_to_i915(m->private);
if (!HAS_FBC(dev_priv)) {
- seq_puts(m, "FBC unsupported on this chipset\n");
+ seq_puts(m, "not supported\n");
return 0;
}
@@ -1809,7 +1809,7 @@ static int i915_ring_freq_table(struct
seq_file *m, void *unused)
unsigned int max_gpu_freq, min_gpu_freq;
if (!HAS_LLC(dev_priv)) {
- seq_puts(m, "unsupported on this chipset\n");
+ seq_puts(m, "not supported\n");
return 0;
}
@@ -2361,8 +2361,10 @@ static int i915_huc_load_status_info(struct
seq_file *m, void *data)
struct drm_i915_private *dev_priv = node_to_i915(m->private);
struct drm_printer p;
- if (!HAS_HUC_UCODE(dev_priv))
+ if (!HAS_GUC(dev_priv)) {
Hmm, I think that in above code we should use HAS_HUC defined as:
/* HuC is inherent part of the GuC ... */
#define HAS_HUC(dev_priv) HAS_GUC(dev_priv)
to make it clear that code checks HuC sub-feature (not other part
of the GuC or GuC itself). And additionally we can use above define
to explicitly document relation between GuC and HuC.
Michal
The suggested comment feels confusing to me. HuC is a different
microcontroller and not a part of GuC. The only tie the 2 have is that
GuC needs to do the authentication. It is however true that any
platform that has a GuC also has a HuC so the suggested define itself
is fine.
Daniele
I agree. I will include the condition and a comment that clearly
mentions the GuC and HuC tie.
+ seq_puts(m, "not supported\n");
return 0;
+ }
p = drm_seq_file_printer(m);
intel_uc_fw_dump(&dev_priv->huc.fw, &p);
@@ -2380,8 +2382,10 @@ static int i915_guc_load_status_info(struct
seq_file *m, void *data)
struct drm_printer p;
u32 tmp, i;
- if (!HAS_GUC_UCODE(dev_priv))
+ if (!HAS_GUC(dev_priv)) {
+ seq_puts(m, "not supported\n");
return 0;
+ }
p = drm_seq_file_printer(m);
intel_uc_fw_dump(&dev_priv->guc.fw, &p);
@@ -2650,7 +2654,7 @@ static int i915_edp_psr_status(struct seq_file
*m, void *data)
bool enabled = false;
if (!HAS_PSR(dev_priv)) {
- seq_puts(m, "PSR not supported\n");
+ seq_puts(m, "not supported\n");
return 0;
}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Thanks for the review.
Regards,
Sujaritha
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
