diff mbox series

[5/8] drm/i915/dmc: don't register DMC debugfs file if there's no DMC

Message ID 99df4792990e8291db420a1a4279afa1c9011074.1647542120.git.jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/dmc: cleanups | expand

Commit Message

Jani Nikula March 17, 2022, 6:36 p.m. UTC
Register the DMC debugfs file only on platforms that support
DMC. There's no point in having a no-op debugfs file.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dmc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Lucas De Marchi March 17, 2022, 7:55 p.m. UTC | #1
On Thu, Mar 17, 2022 at 08:36:17PM +0200, Jani Nikula wrote:
>Register the DMC debugfs file only on platforms that support
>DMC. There's no point in having a no-op debugfs file.

It seems this would not change much the behavior (fail on open vs fail
on read). But the code in igt is suspicious:


	bool igt_pm_dmc_loaded(int debugfs)
	{
		char buf[15];
		int len;

		len = igt_sysfs_read(debugfs, "i915_dmc_info", buf, sizeof(buf) - 1);
		if (len < 0)
			return true; /* no CSR support, no DMC requirement */

 From a quick inspection of igt_sysfs_read() it seems it would just
return 0 if there's nothing to be read. And it would return < 0 on
failure to open the file.

These would be the affected tests:

tests/i915/i915_pm_rpm.c:
tests/i915/i915_pm_lpsp.c:
tests/i915/i915_pm_dc.c:
	igt_require(igt_pm_dmc_loaded(data.debugfs_fd));


Lucas De Marchi

>
>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_dmc.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
>index 5de13f978e57..8dfa2aa9f8bd 100644
>--- a/drivers/gpu/drm/i915/display/intel_dmc.c
>+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
>@@ -818,9 +818,6 @@ static int intel_dmc_debugfs_status_show(struct seq_file *m, void *unused)
> 	struct intel_dmc *dmc;
> 	i915_reg_t dc5_reg, dc6_reg = INVALID_MMIO_REG;
>
>-	if (!HAS_DMC(i915))
>-		return -ENODEV;
>-
> 	dmc = &i915->dmc;
>
> 	wakeref = intel_runtime_pm_get(&i915->runtime_pm);
>@@ -890,6 +887,9 @@ void intel_dmc_debugfs_register(struct drm_i915_private *i915)
> {
> 	struct drm_minor *minor = i915->drm.primary;
>
>+	if (!HAS_DMC(i915))
>+		return;
>+
> 	debugfs_create_file("i915_dmc_info", 0444, minor->debugfs_root,
> 			    i915, &intel_dmc_debugfs_status_fops);
> }
>-- 
>2.30.2
>
Jani Nikula March 18, 2022, 9:22 a.m. UTC | #2
On Thu, 17 Mar 2022, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> On Thu, Mar 17, 2022 at 08:36:17PM +0200, Jani Nikula wrote:
>>Register the DMC debugfs file only on platforms that support
>>DMC. There's no point in having a no-op debugfs file.
>
> It seems this would not change much the behavior (fail on open vs fail
> on read). But the code in igt is suspicious:
>
>
> 	bool igt_pm_dmc_loaded(int debugfs)
> 	{
> 		char buf[15];
> 		int len;
>
> 		len = igt_sysfs_read(debugfs, "i915_dmc_info", buf, sizeof(buf) - 1);
> 		if (len < 0)
> 			return true; /* no CSR support, no DMC requirement */
>
>  From a quick inspection of igt_sysfs_read() it seems it would just
> return 0 if there's nothing to be read. And it would return < 0 on
> failure to open the file.
>
> These would be the affected tests:
>
> tests/i915/i915_pm_rpm.c:
> tests/i915/i915_pm_lpsp.c:
> tests/i915/i915_pm_dc.c:
> 	igt_require(igt_pm_dmc_loaded(data.debugfs_fd));

Ok, I think I'll just drop this patch for now, don't have the time to go
down that rabbit hole...

Thanks,
Jani.

>
>
> Lucas De Marchi
>
>>
>>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>---
>> drivers/gpu/drm/i915/display/intel_dmc.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
>>index 5de13f978e57..8dfa2aa9f8bd 100644
>>--- a/drivers/gpu/drm/i915/display/intel_dmc.c
>>+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
>>@@ -818,9 +818,6 @@ static int intel_dmc_debugfs_status_show(struct seq_file *m, void *unused)
>> 	struct intel_dmc *dmc;
>> 	i915_reg_t dc5_reg, dc6_reg = INVALID_MMIO_REG;
>>
>>-	if (!HAS_DMC(i915))
>>-		return -ENODEV;
>>-
>> 	dmc = &i915->dmc;
>>
>> 	wakeref = intel_runtime_pm_get(&i915->runtime_pm);
>>@@ -890,6 +887,9 @@ void intel_dmc_debugfs_register(struct drm_i915_private *i915)
>> {
>> 	struct drm_minor *minor = i915->drm.primary;
>>
>>+	if (!HAS_DMC(i915))
>>+		return;
>>+
>> 	debugfs_create_file("i915_dmc_info", 0444, minor->debugfs_root,
>> 			    i915, &intel_dmc_debugfs_status_fops);
>> }
>>-- 
>>2.30.2
>>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
index 5de13f978e57..8dfa2aa9f8bd 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -818,9 +818,6 @@  static int intel_dmc_debugfs_status_show(struct seq_file *m, void *unused)
 	struct intel_dmc *dmc;
 	i915_reg_t dc5_reg, dc6_reg = INVALID_MMIO_REG;
 
-	if (!HAS_DMC(i915))
-		return -ENODEV;
-
 	dmc = &i915->dmc;
 
 	wakeref = intel_runtime_pm_get(&i915->runtime_pm);
@@ -890,6 +887,9 @@  void intel_dmc_debugfs_register(struct drm_i915_private *i915)
 {
 	struct drm_minor *minor = i915->drm.primary;
 
+	if (!HAS_DMC(i915))
+		return;
+
 	debugfs_create_file("i915_dmc_info", 0444, minor->debugfs_root,
 			    i915, &intel_dmc_debugfs_status_fops);
 }