Re: [PATCH v2 3/3] drm: Add self_refresh_state debugfs entry

2019-09-18 Thread Daniel Vetter
On Wed, Sep 18, 2019 at 10:07 PM Sean Paul  wrote:
>
> From: Sean Paul 
>
> This patch adds a debugfs entry to surface the entry and exit times as
> well as the calculated delay.
>
> Suggested-by: Daniel Vetter 
> Signed-off-by: Sean Paul 
>
> Changes in v2:
> - Added to the set
> ---
>
> Wasn't too sure how to initialize this, as calling the helper function
> from drm_debugfs.c seemed... wrong. However there weren't any other
> compelling solutions, so I figured I'd post this and learn something
> new.

Won't build, because drm.ko can't depend upon stuff in
drm-kms-helper.ko, since that already depends upon the former.

I think we need a drm_self_refresh_helper_register which drivers can
call from their crtc->late_register callback. I think Noralf was
working on some infrastructure to make this neater, but it didn't land
yet.
-Daniel
>
>
>
>  drivers/gpu/drm/drm_debugfs.c | 10 +
>  drivers/gpu/drm/drm_self_refresh_helper.c | 55 ++-
>  include/drm/drm_self_refresh_helper.h |  6 +++
>  3 files changed, 69 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index eab0f2687cd6..175c2451ae72 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -38,6 +38,9 @@
>  #include 
>  #include 
>  #include 
> +#if defined(CONFIG_DRM_KMS_HELPER)
> +#include 
> +#endif
>
>  #include "drm_crtc_internal.h"
>  #include "drm_internal.h"
> @@ -231,6 +234,13 @@ int drm_debugfs_init(struct drm_minor *minor, int 
> minor_id,
> DRM_ERROR("Failed to create atomic debugfs files\n");
> return ret;
> }
> +#if defined(CONFIG_DRM_KMS_HELPER)
> +   ret = drm_self_refresh_debugfs_init(minor);
> +   if (ret) {
> +   DRM_ERROR("Failed to init self refresh debugfs\n");
> +   return ret;
> +   }
> +#endif
> }
>
> if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> diff --git a/drivers/gpu/drm/drm_self_refresh_helper.c 
> b/drivers/gpu/drm/drm_self_refresh_helper.c
> index 68f4765a5896..e7544ae1e47b 100644
> --- a/drivers/gpu/drm/drm_self_refresh_helper.c
> +++ b/drivers/gpu/drm/drm_self_refresh_helper.c
> @@ -14,7 +14,9 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -167,6 +169,16 @@ void drm_self_refresh_helper_update_avg_times(struct 
> drm_atomic_state *state,
>  }
>  EXPORT_SYMBOL(drm_self_refresh_helper_update_avg_times);
>
> +static unsigned int
> +drm_self_refresh_calc_idle_delay(struct drm_self_refresh_data *sr_data)
> +{
> +   if (WARN_ON(!mutex_is_locked(&sr_data->avg_mutex)))
> +   return SELF_REFRESH_AVG_SEED_MS * 4;
> +
> +   return (ewma_psr_time_read(&sr_data->entry_avg_ms) +
> +   ewma_psr_time_read(&sr_data->exit_avg_ms)) * 2;
> +}
> +
>  /**
>   * drm_self_refresh_helper_alter_state - Alters the atomic state for SR exit
>   * @state: the state currently being checked
> @@ -209,8 +221,7 @@ void drm_self_refresh_helper_alter_state(struct 
> drm_atomic_state *state)
> continue;
>
> mutex_lock(&sr_data->avg_mutex);
> -   delay = (ewma_psr_time_read(&sr_data->entry_avg_ms) +
> -ewma_psr_time_read(&sr_data->exit_avg_ms)) * 2;
> +   delay = drm_self_refresh_calc_idle_delay(sr_data);
> mutex_unlock(&sr_data->avg_mutex);
>
> mod_delayed_work(system_wq, &sr_data->entry_work,
> @@ -275,3 +286,43 @@ void drm_self_refresh_helper_cleanup(struct drm_crtc 
> *crtc)
> kfree(sr_data);
>  }
>  EXPORT_SYMBOL(drm_self_refresh_helper_cleanup);
> +
> +#ifdef CONFIG_DEBUG_FS
> +
> +static int drm_self_refresh_debugfs_state(struct seq_file *m, void *data)
> +{
> +   struct drm_info_node *node = (struct drm_info_node *) m->private;
> +   struct drm_device *dev = node->minor->dev;
> +   struct drm_printer p = drm_seq_file_printer(m);
> +   struct drm_crtc *crtc;
> +
> +   drm_for_each_crtc(crtc, dev) {
> +   struct drm_self_refresh_data *sr_data = 
> crtc->self_refresh_data;
> +   if (!sr_data)
> +   continue;
> +
> +   mutex_lock(&sr_data->avg_mutex);
> +   drm_printf(&p, "crtc[%u]: %s\n", crtc->base.id, crtc->name);
> +   drm_printf(&p, "\tentry_avg_ms=%lu\n",
> +  ewma_psr_time_read(&sr_data->entry_avg_ms));
> +   drm_printf(&p, "\texit_avg_ms=%lu\n",
> +  ewma_psr_time_read(&sr_data->exit_avg_ms));
> +   drm_printf(&p, "\tidle_delay=%u\n",
> +  drm_self_refresh_calc_idle_delay(sr_data));
> +   mutex_unlock(&sr_data->avg_mutex);
> +   }
> +   return 0;
> +}
> +
> +static const struct drm_info_list drm_atomic_debugfs_list[]

[PATCH v2 3/3] drm: Add self_refresh_state debugfs entry

2019-09-18 Thread Sean Paul
From: Sean Paul 

This patch adds a debugfs entry to surface the entry and exit times as
well as the calculated delay.

Suggested-by: Daniel Vetter 
Signed-off-by: Sean Paul 

Changes in v2:
- Added to the set
---

Wasn't too sure how to initialize this, as calling the helper function
from drm_debugfs.c seemed... wrong. However there weren't any other
compelling solutions, so I figured I'd post this and learn something
new.



 drivers/gpu/drm/drm_debugfs.c | 10 +
 drivers/gpu/drm/drm_self_refresh_helper.c | 55 ++-
 include/drm/drm_self_refresh_helper.h |  6 +++
 3 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index eab0f2687cd6..175c2451ae72 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -38,6 +38,9 @@
 #include 
 #include 
 #include 
+#if defined(CONFIG_DRM_KMS_HELPER)
+#include 
+#endif
 
 #include "drm_crtc_internal.h"
 #include "drm_internal.h"
@@ -231,6 +234,13 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
DRM_ERROR("Failed to create atomic debugfs files\n");
return ret;
}
+#if defined(CONFIG_DRM_KMS_HELPER)
+   ret = drm_self_refresh_debugfs_init(minor);
+   if (ret) {
+   DRM_ERROR("Failed to init self refresh debugfs\n");
+   return ret;
+   }
+#endif
}
 
if (drm_core_check_feature(dev, DRIVER_MODESET)) {
diff --git a/drivers/gpu/drm/drm_self_refresh_helper.c 
b/drivers/gpu/drm/drm_self_refresh_helper.c
index 68f4765a5896..e7544ae1e47b 100644
--- a/drivers/gpu/drm/drm_self_refresh_helper.c
+++ b/drivers/gpu/drm/drm_self_refresh_helper.c
@@ -14,7 +14,9 @@
 #include 
 #include 
 #include 
+#include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -167,6 +169,16 @@ void drm_self_refresh_helper_update_avg_times(struct 
drm_atomic_state *state,
 }
 EXPORT_SYMBOL(drm_self_refresh_helper_update_avg_times);
 
+static unsigned int
+drm_self_refresh_calc_idle_delay(struct drm_self_refresh_data *sr_data)
+{
+   if (WARN_ON(!mutex_is_locked(&sr_data->avg_mutex)))
+   return SELF_REFRESH_AVG_SEED_MS * 4;
+
+   return (ewma_psr_time_read(&sr_data->entry_avg_ms) +
+   ewma_psr_time_read(&sr_data->exit_avg_ms)) * 2;
+}
+
 /**
  * drm_self_refresh_helper_alter_state - Alters the atomic state for SR exit
  * @state: the state currently being checked
@@ -209,8 +221,7 @@ void drm_self_refresh_helper_alter_state(struct 
drm_atomic_state *state)
continue;
 
mutex_lock(&sr_data->avg_mutex);
-   delay = (ewma_psr_time_read(&sr_data->entry_avg_ms) +
-ewma_psr_time_read(&sr_data->exit_avg_ms)) * 2;
+   delay = drm_self_refresh_calc_idle_delay(sr_data);
mutex_unlock(&sr_data->avg_mutex);
 
mod_delayed_work(system_wq, &sr_data->entry_work,
@@ -275,3 +286,43 @@ void drm_self_refresh_helper_cleanup(struct drm_crtc *crtc)
kfree(sr_data);
 }
 EXPORT_SYMBOL(drm_self_refresh_helper_cleanup);
+
+#ifdef CONFIG_DEBUG_FS
+
+static int drm_self_refresh_debugfs_state(struct seq_file *m, void *data)
+{
+   struct drm_info_node *node = (struct drm_info_node *) m->private;
+   struct drm_device *dev = node->minor->dev;
+   struct drm_printer p = drm_seq_file_printer(m);
+   struct drm_crtc *crtc;
+
+   drm_for_each_crtc(crtc, dev) {
+   struct drm_self_refresh_data *sr_data = crtc->self_refresh_data;
+   if (!sr_data)
+   continue;
+
+   mutex_lock(&sr_data->avg_mutex);
+   drm_printf(&p, "crtc[%u]: %s\n", crtc->base.id, crtc->name);
+   drm_printf(&p, "\tentry_avg_ms=%lu\n",
+  ewma_psr_time_read(&sr_data->entry_avg_ms));
+   drm_printf(&p, "\texit_avg_ms=%lu\n",
+  ewma_psr_time_read(&sr_data->exit_avg_ms));
+   drm_printf(&p, "\tidle_delay=%u\n",
+  drm_self_refresh_calc_idle_delay(sr_data));
+   mutex_unlock(&sr_data->avg_mutex);
+   }
+   return 0;
+}
+
+static const struct drm_info_list drm_atomic_debugfs_list[] = {
+   {"self_refresh_state", drm_self_refresh_debugfs_state, 0},
+};
+
+int drm_self_refresh_debugfs_init(struct drm_minor *minor)
+{
+   return drm_debugfs_create_files(drm_atomic_debugfs_list,
+   ARRAY_SIZE(drm_atomic_debugfs_list),
+   minor->debugfs_root, minor);
+}
+EXPORT_SYMBOL(drm_self_refresh_debugfs_init);
+#endif
diff --git a/include/drm/drm_self_refresh_helper.h 
b/include/drm/drm_self_refresh_helper.h
index 5b79d253fb46..779f81e035f1 100644
--- a/include/drm/drm_self_refresh_helper.h
+++ b/include/drm/drm_self_refresh_helper.h
@@ -17,4 +17,10 @@ void drm_self_refresh_h