diff mbox series

drm/i915/pps: move pps debugfs file to intel_pps.c

Message ID 20240408094357.3085319-1-jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/pps: move pps debugfs file to intel_pps.c | expand

Commit Message

Jani Nikula April 8, 2024, 9:43 a.m. UTC
Continue with placing debugfs next to the implementation.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 .../drm/i915/display/intel_display_debugfs.c  | 27 ++--------------
 drivers/gpu/drm/i915/display/intel_pps.c      | 32 +++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_pps.h      |  2 ++
 3 files changed, 36 insertions(+), 25 deletions(-)

Comments

Ville Syrjälä April 8, 2024, 12:13 p.m. UTC | #1
On Mon, Apr 08, 2024 at 12:43:57PM +0300, Jani Nikula wrote:
> Continue with placing debugfs next to the implementation.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  .../drm/i915/display/intel_display_debugfs.c  | 27 ++--------------
>  drivers/gpu/drm/i915/display/intel_pps.c      | 32 +++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_pps.h      |  2 ++
>  3 files changed, 36 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index 5235f8758ef1..0feffe8d4e45 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -31,6 +31,7 @@
>  #include "intel_hdmi.h"
>  #include "intel_hotplug.h"
>  #include "intel_panel.h"
> +#include "intel_pps.h"
>  #include "intel_psr.h"
>  #include "intel_psr_regs.h"
>  #include "intel_wm.h"
> @@ -1095,27 +1096,6 @@ void intel_display_debugfs_register(struct drm_i915_private *i915)
>  	intel_display_debugfs_params(i915);
>  }
>  
> -static int i915_panel_show(struct seq_file *m, void *data)
> -{
> -	struct intel_connector *connector = m->private;
> -	struct intel_dp *intel_dp = intel_attached_dp(connector);
> -
> -	if (connector->base.status != connector_status_connected)
> -		return -ENODEV;
> -
> -	seq_printf(m, "Panel power up delay: %d\n",
> -		   intel_dp->pps.panel_power_up_delay);
> -	seq_printf(m, "Panel power down delay: %d\n",
> -		   intel_dp->pps.panel_power_down_delay);
> -	seq_printf(m, "Backlight on delay: %d\n",
> -		   intel_dp->pps.backlight_on_delay);
> -	seq_printf(m, "Backlight off delay: %d\n",
> -		   intel_dp->pps.backlight_off_delay);
> -
> -	return 0;
> -}
> -DEFINE_SHOW_ATTRIBUTE(i915_panel);
> -
>  static int i915_hdcp_sink_capability_show(struct seq_file *m, void *data)
>  {
>  	struct intel_connector *connector = m->private;
> @@ -1560,12 +1540,9 @@ void intel_connector_debugfs_add(struct intel_connector *connector)
>  		return;
>  
>  	intel_drrs_connector_debugfs_add(connector);
> +	intel_pps_connector_debugfs_add(connector);
>  	intel_psr_connector_debugfs_add(connector);
>  
> -	if (connector_type == DRM_MODE_CONNECTOR_eDP)
> -		debugfs_create_file("i915_panel_timings", 0444, root,
> -				    connector, &i915_panel_fops);
> -
>  	if (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
>  	    connector_type == DRM_MODE_CONNECTOR_HDMIA ||
>  	    connector_type == DRM_MODE_CONNECTOR_HDMIB) {
> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
> index b5d9920f8341..88a44d93f82b 100644
> --- a/drivers/gpu/drm/i915/display/intel_pps.c
> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
> @@ -1670,6 +1670,38 @@ void intel_pps_setup(struct drm_i915_private *i915)
>  		i915->display.pps.mmio_base = PPS_BASE;
>  }
>  
> +static int intel_pps_show(struct seq_file *m, void *data)
> +{
> +	struct intel_connector *connector = m->private;
> +	struct intel_dp *intel_dp = intel_attached_dp(connector);
> +
> +	if (connector->base.status != connector_status_connected)
> +		return -ENODEV;

That check seems completely pointless. Could be removed as a
followup.

Hmm. The other question that comes to mind is whether anyone has
ever used this file? I for sure have not. So I'm wondering if we
could just nuke the whole thing?

Anyways
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +
> +	seq_printf(m, "Panel power up delay: %d\n",
> +		   intel_dp->pps.panel_power_up_delay);
> +	seq_printf(m, "Panel power down delay: %d\n",
> +		   intel_dp->pps.panel_power_down_delay);
> +	seq_printf(m, "Backlight on delay: %d\n",
> +		   intel_dp->pps.backlight_on_delay);
> +	seq_printf(m, "Backlight off delay: %d\n",
> +		   intel_dp->pps.backlight_off_delay);
> +
> +	return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(intel_pps);
> +
> +void intel_pps_connector_debugfs_add(struct intel_connector *connector)
> +{
> +	struct dentry *root = connector->base.debugfs_entry;
> +	int connector_type = connector->base.connector_type;
> +
> +	if (connector_type == DRM_MODE_CONNECTOR_eDP)
> +		debugfs_create_file("i915_panel_timings", 0444, root,
> +				    connector, &intel_pps_fops);
> +
> +}
> +
>  void assert_pps_unlocked(struct drm_i915_private *dev_priv, enum pipe pipe)
>  {
>  	i915_reg_t pp_reg;
> diff --git a/drivers/gpu/drm/i915/display/intel_pps.h b/drivers/gpu/drm/i915/display/intel_pps.h
> index a2c2467e3c22..07ef96ca8da2 100644
> --- a/drivers/gpu/drm/i915/display/intel_pps.h
> +++ b/drivers/gpu/drm/i915/display/intel_pps.h
> @@ -51,6 +51,8 @@ void vlv_pps_init(struct intel_encoder *encoder,
>  void intel_pps_unlock_regs_wa(struct drm_i915_private *i915);
>  void intel_pps_setup(struct drm_i915_private *i915);
>  
> +void intel_pps_connector_debugfs_add(struct intel_connector *connector);
> +
>  void assert_pps_unlocked(struct drm_i915_private *i915, enum pipe pipe);
>  
>  #endif /* __INTEL_PPS_H__ */
> -- 
> 2.39.2
Jani Nikula April 10, 2024, 11:38 a.m. UTC | #2
On Mon, 08 Apr 2024, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Mon, Apr 08, 2024 at 12:43:57PM +0300, Jani Nikula wrote:
>> Continue with placing debugfs next to the implementation.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  .../drm/i915/display/intel_display_debugfs.c  | 27 ++--------------
>>  drivers/gpu/drm/i915/display/intel_pps.c      | 32 +++++++++++++++++++
>>  drivers/gpu/drm/i915/display/intel_pps.h      |  2 ++
>>  3 files changed, 36 insertions(+), 25 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>> index 5235f8758ef1..0feffe8d4e45 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>> @@ -31,6 +31,7 @@
>>  #include "intel_hdmi.h"
>>  #include "intel_hotplug.h"
>>  #include "intel_panel.h"
>> +#include "intel_pps.h"
>>  #include "intel_psr.h"
>>  #include "intel_psr_regs.h"
>>  #include "intel_wm.h"
>> @@ -1095,27 +1096,6 @@ void intel_display_debugfs_register(struct drm_i915_private *i915)
>>  	intel_display_debugfs_params(i915);
>>  }
>>  
>> -static int i915_panel_show(struct seq_file *m, void *data)
>> -{
>> -	struct intel_connector *connector = m->private;
>> -	struct intel_dp *intel_dp = intel_attached_dp(connector);
>> -
>> -	if (connector->base.status != connector_status_connected)
>> -		return -ENODEV;
>> -
>> -	seq_printf(m, "Panel power up delay: %d\n",
>> -		   intel_dp->pps.panel_power_up_delay);
>> -	seq_printf(m, "Panel power down delay: %d\n",
>> -		   intel_dp->pps.panel_power_down_delay);
>> -	seq_printf(m, "Backlight on delay: %d\n",
>> -		   intel_dp->pps.backlight_on_delay);
>> -	seq_printf(m, "Backlight off delay: %d\n",
>> -		   intel_dp->pps.backlight_off_delay);
>> -
>> -	return 0;
>> -}
>> -DEFINE_SHOW_ATTRIBUTE(i915_panel);
>> -
>>  static int i915_hdcp_sink_capability_show(struct seq_file *m, void *data)
>>  {
>>  	struct intel_connector *connector = m->private;
>> @@ -1560,12 +1540,9 @@ void intel_connector_debugfs_add(struct intel_connector *connector)
>>  		return;
>>  
>>  	intel_drrs_connector_debugfs_add(connector);
>> +	intel_pps_connector_debugfs_add(connector);
>>  	intel_psr_connector_debugfs_add(connector);
>>  
>> -	if (connector_type == DRM_MODE_CONNECTOR_eDP)
>> -		debugfs_create_file("i915_panel_timings", 0444, root,
>> -				    connector, &i915_panel_fops);
>> -
>>  	if (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
>>  	    connector_type == DRM_MODE_CONNECTOR_HDMIA ||
>>  	    connector_type == DRM_MODE_CONNECTOR_HDMIB) {
>> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
>> index b5d9920f8341..88a44d93f82b 100644
>> --- a/drivers/gpu/drm/i915/display/intel_pps.c
>> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
>> @@ -1670,6 +1670,38 @@ void intel_pps_setup(struct drm_i915_private *i915)
>>  		i915->display.pps.mmio_base = PPS_BASE;
>>  }
>>  
>> +static int intel_pps_show(struct seq_file *m, void *data)
>> +{
>> +	struct intel_connector *connector = m->private;
>> +	struct intel_dp *intel_dp = intel_attached_dp(connector);
>> +
>> +	if (connector->base.status != connector_status_connected)
>> +		return -ENODEV;
>
> That check seems completely pointless. Could be removed as a
> followup.
>
> Hmm. The other question that comes to mind is whether anyone has
> ever used this file? I for sure have not. So I'm wondering if we
> could just nuke the whole thing?

Good question. In the mean time, with this patch at hand, pushed to din.

>
> Anyways
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks!

BR,
Jani.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index 5235f8758ef1..0feffe8d4e45 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -31,6 +31,7 @@ 
 #include "intel_hdmi.h"
 #include "intel_hotplug.h"
 #include "intel_panel.h"
+#include "intel_pps.h"
 #include "intel_psr.h"
 #include "intel_psr_regs.h"
 #include "intel_wm.h"
@@ -1095,27 +1096,6 @@  void intel_display_debugfs_register(struct drm_i915_private *i915)
 	intel_display_debugfs_params(i915);
 }
 
-static int i915_panel_show(struct seq_file *m, void *data)
-{
-	struct intel_connector *connector = m->private;
-	struct intel_dp *intel_dp = intel_attached_dp(connector);
-
-	if (connector->base.status != connector_status_connected)
-		return -ENODEV;
-
-	seq_printf(m, "Panel power up delay: %d\n",
-		   intel_dp->pps.panel_power_up_delay);
-	seq_printf(m, "Panel power down delay: %d\n",
-		   intel_dp->pps.panel_power_down_delay);
-	seq_printf(m, "Backlight on delay: %d\n",
-		   intel_dp->pps.backlight_on_delay);
-	seq_printf(m, "Backlight off delay: %d\n",
-		   intel_dp->pps.backlight_off_delay);
-
-	return 0;
-}
-DEFINE_SHOW_ATTRIBUTE(i915_panel);
-
 static int i915_hdcp_sink_capability_show(struct seq_file *m, void *data)
 {
 	struct intel_connector *connector = m->private;
@@ -1560,12 +1540,9 @@  void intel_connector_debugfs_add(struct intel_connector *connector)
 		return;
 
 	intel_drrs_connector_debugfs_add(connector);
+	intel_pps_connector_debugfs_add(connector);
 	intel_psr_connector_debugfs_add(connector);
 
-	if (connector_type == DRM_MODE_CONNECTOR_eDP)
-		debugfs_create_file("i915_panel_timings", 0444, root,
-				    connector, &i915_panel_fops);
-
 	if (connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
 	    connector_type == DRM_MODE_CONNECTOR_HDMIA ||
 	    connector_type == DRM_MODE_CONNECTOR_HDMIB) {
diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c
index b5d9920f8341..88a44d93f82b 100644
--- a/drivers/gpu/drm/i915/display/intel_pps.c
+++ b/drivers/gpu/drm/i915/display/intel_pps.c
@@ -1670,6 +1670,38 @@  void intel_pps_setup(struct drm_i915_private *i915)
 		i915->display.pps.mmio_base = PPS_BASE;
 }
 
+static int intel_pps_show(struct seq_file *m, void *data)
+{
+	struct intel_connector *connector = m->private;
+	struct intel_dp *intel_dp = intel_attached_dp(connector);
+
+	if (connector->base.status != connector_status_connected)
+		return -ENODEV;
+
+	seq_printf(m, "Panel power up delay: %d\n",
+		   intel_dp->pps.panel_power_up_delay);
+	seq_printf(m, "Panel power down delay: %d\n",
+		   intel_dp->pps.panel_power_down_delay);
+	seq_printf(m, "Backlight on delay: %d\n",
+		   intel_dp->pps.backlight_on_delay);
+	seq_printf(m, "Backlight off delay: %d\n",
+		   intel_dp->pps.backlight_off_delay);
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(intel_pps);
+
+void intel_pps_connector_debugfs_add(struct intel_connector *connector)
+{
+	struct dentry *root = connector->base.debugfs_entry;
+	int connector_type = connector->base.connector_type;
+
+	if (connector_type == DRM_MODE_CONNECTOR_eDP)
+		debugfs_create_file("i915_panel_timings", 0444, root,
+				    connector, &intel_pps_fops);
+
+}
+
 void assert_pps_unlocked(struct drm_i915_private *dev_priv, enum pipe pipe)
 {
 	i915_reg_t pp_reg;
diff --git a/drivers/gpu/drm/i915/display/intel_pps.h b/drivers/gpu/drm/i915/display/intel_pps.h
index a2c2467e3c22..07ef96ca8da2 100644
--- a/drivers/gpu/drm/i915/display/intel_pps.h
+++ b/drivers/gpu/drm/i915/display/intel_pps.h
@@ -51,6 +51,8 @@  void vlv_pps_init(struct intel_encoder *encoder,
 void intel_pps_unlock_regs_wa(struct drm_i915_private *i915);
 void intel_pps_setup(struct drm_i915_private *i915);
 
+void intel_pps_connector_debugfs_add(struct intel_connector *connector);
+
 void assert_pps_unlocked(struct drm_i915_private *i915, enum pipe pipe);
 
 #endif /* __INTEL_PPS_H__ */