diff mbox

[v5,1/5] drm/i915/guc : Unifying seq_puts messages

Message ID 1507071341-23126-1-git-send-email-sujaritha.sundaresan@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sundaresan, Sujaritha Oct. 3, 2017, 10:55 p.m. UTC
Unifying the various seq_puts messages to the simplest one

v2: Clarifying the commit message (Anusha)

v3: Unify seq_puts messages, Re-factoring code as per review (Michal)

v4: Rebase

v5: Separated into a separate patch

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

sagar.a.kamble@intel.com Oct. 4, 2017, 6:13 a.m. UTC | #1
On 10/4/2017 4:25 AM, Sujaritha Sundaresan wrote:
> Unifying the various seq_puts messages to the simplest one
>
> v2: Clarifying the commit message (Anusha)
>
> v3: Unify seq_puts messages, Re-factoring code as per review (Michal)
>
> v4: Rebase
>
> v5: Separated into a separate patch
>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 847f8e8..53e40dd 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1616,7 +1616,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;
>   	}
>   
> @@ -1783,7 +1783,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;
>   	}
>   
> @@ -2654,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;
>   	}
>   
> @@ -2807,7 +2807,7 @@ static int i915_runtime_pm_status(struct seq_file *m, void *unused)
>   	struct pci_dev *pdev = dev_priv->drm.pdev;
>   
>   	if (!HAS_RUNTIME_PM(dev_priv))
> -		seq_puts(m, "Runtime power management not supported\n");
> +		seq_puts(m, "not supported\n");
>   
>   	seq_printf(m, "GPU idle: %s\n", yesno(!dev_priv->gt.awake));
>   	seq_printf(m, "IRQs disabled: %s\n",
> @@ -3683,7 +3683,7 @@ static void drrs_status_per_crtc(struct seq_file *m,
>   		mutex_unlock(&drrs->mutex);
>   	} else {
>   		/* DRRS not supported. Print the VBT parameter*/
> -		seq_puts(m, "\tDRRS Supported : No");
> +		seq_puts(m, "not supported\n");
There is corresponding "Yes" seq_puts above. Please update that as well.
Commit subject looks little ambiguous. can we say unify seq_puts 
messages for feature support.
>   	}
>   	seq_puts(m, "\n");
>   }
Michal Wajdeczko Oct. 4, 2017, 11:39 a.m. UTC | #2
On Wed, 04 Oct 2017 08:13:12 +0200, Sagar Arun Kamble  
<sagar.a.kamble@intel.com> wrote:

>
>
> On 10/4/2017 4:25 AM, Sujaritha Sundaresan wrote:
>> Unifying the various seq_puts messages to the simplest one
>>
>> v2: Clarifying the commit message (Anusha)
>>
>> v3: Unify seq_puts messages, Re-factoring code as per review (Michal)
>>
>> v4: Rebase
>>
>> v5: Separated into a separate patch
>>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
>> Cc: Oscar Mateo <oscar.mateo@intel.com>
>> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_debugfs.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c  
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 847f8e8..53e40dd 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -1616,7 +1616,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;
>>   	}
>>   @@ -1783,7 +1783,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;
>>   	}
>>   @@ -2654,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;
>>   	}
>>   @@ -2807,7 +2807,7 @@ static int i915_runtime_pm_status(struct  
>> seq_file *m, void *unused)
>>   	struct pci_dev *pdev = dev_priv->drm.pdev;
>>     	if (!HAS_RUNTIME_PM(dev_priv))
>> -		seq_puts(m, "Runtime power management not supported\n");
>> +		seq_puts(m, "not supported\n");

This one also does not fit into unified "early return" pattern.

>>     	seq_printf(m, "GPU idle: %s\n", yesno(!dev_priv->gt.awake));
>>   	seq_printf(m, "IRQs disabled: %s\n",
>> @@ -3683,7 +3683,7 @@ static void drrs_status_per_crtc(struct seq_file  
>> *m,
>>   		mutex_unlock(&drrs->mutex);
>>   	} else {
>>   		/* DRRS not supported. Print the VBT parameter*/
>> -		seq_puts(m, "\tDRRS Supported : No");
>> +		seq_puts(m, "not supported\n");
> There is corresponding "Yes" seq_puts above. Please update that as well.
> Commit subject looks little ambiguous. can we say unify seq_puts  
> messages for feature support.
>>   	}
>>   	seq_puts(m, "\n");
>>   }
Sundaresan, Sujaritha Oct. 4, 2017, 5:23 p.m. UTC | #3
On 10/03/2017 11:13 PM, Sagar Arun Kamble wrote:
>
>
> On 10/4/2017 4:25 AM, Sujaritha Sundaresan wrote:
>> Unifying the various seq_puts messages to the simplest one
>>
>> v2: Clarifying the commit message (Anusha)
>>
>> v3: Unify seq_puts messages, Re-factoring code as per review (Michal)
>>
>> v4: Rebase
>>
>> v5: Separated into a separate patch
>>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
>> Cc: Oscar Mateo <oscar.mateo@intel.com>
>> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_debugfs.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 847f8e8..53e40dd 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -1616,7 +1616,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;
>>       }
>>   @@ -1783,7 +1783,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;
>>       }
>>   @@ -2654,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;
>>       }
>>   @@ -2807,7 +2807,7 @@ static int i915_runtime_pm_status(struct 
>> seq_file *m, void *unused)
>>       struct pci_dev *pdev = dev_priv->drm.pdev;
>>         if (!HAS_RUNTIME_PM(dev_priv))
>> -        seq_puts(m, "Runtime power management not supported\n");
>> +        seq_puts(m, "not supported\n");
>>         seq_printf(m, "GPU idle: %s\n", yesno(!dev_priv->gt.awake));
>>       seq_printf(m, "IRQs disabled: %s\n",
>> @@ -3683,7 +3683,7 @@ static void drrs_status_per_crtc(struct 
>> seq_file *m,
>>           mutex_unlock(&drrs->mutex);
>>       } else {
>>           /* DRRS not supported. Print the VBT parameter*/
>> -        seq_puts(m, "\tDRRS Supported : No");
>> +        seq_puts(m, "not supported\n");
> There is corresponding "Yes" seq_puts above. Please update that as well.

Will do.
> Commit subject looks little ambiguous. can we say unify seq_puts 
> messages for feature support.

Yes, I will clarify the commit message.
>>       }
>>       seq_puts(m, "\n");
>>   }
>

Thanks for the review,

Regards,

Sujaritha
Sundaresan, Sujaritha Oct. 4, 2017, 5:29 p.m. UTC | #4
On 10/04/2017 04:39 AM, Michal Wajdeczko wrote:
> On Wed, 04 Oct 2017 08:13:12 +0200, Sagar Arun Kamble 
> <sagar.a.kamble@intel.com> wrote:
>
>>
>>
>> On 10/4/2017 4:25 AM, Sujaritha Sundaresan wrote:
>>> Unifying the various seq_puts messages to the simplest one
>>>
>>> v2: Clarifying the commit message (Anusha)
>>>
>>> v3: Unify seq_puts messages, Re-factoring code as per review (Michal)
>>>
>>> v4: Rebase
>>>
>>> v5: Separated into a separate patch
>>>
>>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
>>> Cc: Oscar Mateo <oscar.mateo@intel.com>
>>> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>>> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/i915_debugfs.c | 10 +++++-----
>>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
>>> b/drivers/gpu/drm/i915/i915_debugfs.c
>>> index 847f8e8..53e40dd 100644
>>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>>> @@ -1616,7 +1616,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;
>>>       }
>>>   @@ -1783,7 +1783,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;
>>>       }
>>>   @@ -2654,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;
>>>       }
>>>   @@ -2807,7 +2807,7 @@ static int i915_runtime_pm_status(struct 
>>> seq_file *m, void *unused)
>>>       struct pci_dev *pdev = dev_priv->drm.pdev;
>>>         if (!HAS_RUNTIME_PM(dev_priv))
>>> -        seq_puts(m, "Runtime power management not supported\n");
>>> +        seq_puts(m, "not supported\n");
>
> This one also does not fit into unified "early return" pattern.

Will revert this one.

>
>>>         seq_printf(m, "GPU idle: %s\n", yesno(!dev_priv->gt.awake));
>>>       seq_printf(m, "IRQs disabled: %s\n",
>>> @@ -3683,7 +3683,7 @@ static void drrs_status_per_crtc(struct 
>>> seq_file *m,
>>>           mutex_unlock(&drrs->mutex);
>>>       } else {
>>>           /* DRRS not supported. Print the VBT parameter*/
>>> -        seq_puts(m, "\tDRRS Supported : No");
>>> +        seq_puts(m, "not supported\n");
>> There is corresponding "Yes" seq_puts above. Please update that as well.
>> Commit subject looks little ambiguous. can we say unify seq_puts 
>> messages for feature support.
>>>       }
>>>       seq_puts(m, "\n");
>>>   }
  Thanks for the review.

Regards,

Sujaritha
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 847f8e8..53e40dd 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1616,7 +1616,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;
 	}
 
@@ -1783,7 +1783,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;
 	}
 
@@ -2654,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;
 	}
 
@@ -2807,7 +2807,7 @@  static int i915_runtime_pm_status(struct seq_file *m, void *unused)
 	struct pci_dev *pdev = dev_priv->drm.pdev;
 
 	if (!HAS_RUNTIME_PM(dev_priv))
-		seq_puts(m, "Runtime power management not supported\n");
+		seq_puts(m, "not supported\n");
 
 	seq_printf(m, "GPU idle: %s\n", yesno(!dev_priv->gt.awake));
 	seq_printf(m, "IRQs disabled: %s\n",
@@ -3683,7 +3683,7 @@  static void drrs_status_per_crtc(struct seq_file *m,
 		mutex_unlock(&drrs->mutex);
 	} else {
 		/* DRRS not supported. Print the VBT parameter*/
-		seq_puts(m, "\tDRRS Supported : No");
+		seq_puts(m, "not supported\n");
 	}
 	seq_puts(m, "\n");
 }