On Tue, 29 Mar 2022, "Dixit, Ashutosh" <ashutosh.di...@intel.com> wrote:
> On Mon, 28 Mar 2022 03:22:27 -0700, Anshuman Gupta wrote:
>>
>> +#ifdef CONFIG_PM
>> +static int i915_runtime_dump_child_status(struct device *dev, void *data)
>> +{
>> +    struct seq_file *m = data;
>> +    const char *rpm_status;
>> +
>> +    /* Early return if runtime_pm is disabled */
>> +    if (dev->power.disable_depth)
>> +            return 0;
>> +
>> +    switch (dev->power.runtime_status) {
>> +    case RPM_SUSPENDED:
>> +            rpm_status = "suspended";
>> +            break;
>> +    case RPM_SUSPENDING:
>> +            rpm_status = "suspending";
>> +            break;
>> +    case RPM_RESUMING:
>> +            rpm_status = "resuming";
>> +            break;
>> +    case RPM_ACTIVE:
>> +            rpm_status = "active";
>> +            break;
>> +    default:
>> +            rpm_status = "unknown";
>> +    }
>> +
>> +    seq_printf(m, "\t%s %s: Runtime status: %s\n", dev_driver_string(dev),
>> +               dev_name(dev), rpm_status);
>> +
>> +    return 0;
>> +}
>> +#endif
>
> Maybe a nit, but perhaps defining a const array is better than having a
> switch statement? Similar to what is done in rtpm_status_str(). The
> function itself is very similar to rtpm_status_str() so can probably
> benefit from that similarity. Can perhaps even be nearly identical to
> rtpm_status_str() (since that is static in the genpd (generic power domain)
> code).
>
> See also 2bd5306a8764 ("PM / Domains: add debugfs listing of struct
> generic_pm_domain-s"), though I am not sure if genpd's are applicable in
> our case and certainly look way out of scope for now. Thanks.

See also /sys/devices/i915/power/runtime_status and
/sys/devices/i915/power/runtime_active_kids.

Kinda feels like the info should be made available there?

BR,
Jani.

>
>> +
>>  static int i915_runtime_pm_status(struct seq_file *m, void *unused)
>>  {
>>      struct drm_i915_private *dev_priv = node_to_i915(m->private);
>> @@ -500,6 +534,10 @@ static int i915_runtime_pm_status(struct seq_file *m, 
>> void *unused)
>>  #ifdef CONFIG_PM
>>      seq_printf(m, "Usage count: %d\n",
>>                 atomic_read(&dev_priv->drm.dev->power.usage_count));
>> +    seq_printf(m, "Runtime active children: %d\n",
>> +               atomic_read(&dev_priv->drm.dev->power.child_count));
>> +    device_for_each_child(&pdev->dev, m, i915_runtime_dump_child_status);
>> +
>>  #else
>>      seq_printf(m, "Device Power Management (CONFIG_PM) disabled\n");
>>  #endif
>> --
>> 2.26.2
>>

-- 
Jani Nikula, Intel Open Source Graphics Center

Reply via email to